RFC: Patch for Bug#749 (sun.applet.PluginStreamHandler#handleMessage(String) really slow)
Dr Andrew John Hughes
ahughes at redhat.com
Wed Jul 20 14:18:45 PDT 2011
On 11:16 Tue 05 Jul , Deepak Bhole wrote:
> * Andrew John Hughes <ahughes at redhat.com> [2011-07-04 21:48]:
> > On Mon, Jul 04, 2011 at 06:35:14PM -0400, Deepak Bhole wrote:
> > > Hi,
> > >
> > > This is a cleaned up version of the patch posted for Bug# 749:
> > > http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=749
> > >
> > > I have reviewed the patch myself and found no issues. The patch is
> > > fairly straightforward and if there are no emails by tomorrow,
> > > I will go ahead and commit to 1.0, 1.1 and HEAD.
> > >
> > > Cheers,
> > > Deepak
> >
> > I've commented inline. The code itself looks ok, but it needs to be
> > better documented.
> >
> > However, your comment about applying this patch, given no response
> > disturbs me. We don't have a policy of doing this and I strongly
> > think we should not do so. Please wait for a review, especially
> > for release branch patches. You can't take no response after some
> > short period (and 'tomorrow' is quite vague) as implicit approval
> > of your patch. People do have other stuff to work on.
> >
>
> The fix looked quite harmless so I didn't think much of it.
Yeah, the point wasn't about the fix itself, but more a general point
about submitting patches.
> Nonetheless,
> I will wait for approval next time.
>
Thanks.
> New patch with updated docs attached. Thanks for taking a look!
>
Thanks. Probably best to delete the old one next time to avoid confusion, though
I admit the old comments are useful :-)
Approved (not before time... guess it got lost in the mail outage).
> Cheers,
> Deepak
>
> diff -r 86abbf8be0b1 AUTHORS
> --- a/AUTHORS Thu Jun 23 15:29:45 2011 +0200
> +++ b/AUTHORS Tue Jul 05 11:12:20 2011 -0400
> @@ -3,6 +3,7 @@
>
> Lillian Angel <langel at redhat.com>
> Deepak Bhole <dbhole at redhat.com>
> +Ricardo Martín Camarero <rickyepoderi at yahoo.es>
> Thomas Fitzsimmons <fitzsim at redhat.com>
> Mark Greenwood <mark at dcs.shef.ac.uk>
> Andrew John Hughes <gnu_andrew at member.fsf.org, ahughes at redhat.com>
> diff -r 86abbf8be0b1 ChangeLog
> --- a/ChangeLog Thu Jun 23 15:29:45 2011 +0200
> +++ b/ChangeLog Tue Jul 05 11:12:20 2011 -0400
> @@ -1,3 +1,12 @@
> +2011-07-05 Deepak Bhole <dbhole at redhat.com>
> +
> + PR749: sun.applet.PluginStreamHandler#handleMessage(String) really slow
> + Patch from: Ricardo Martín Camarero (Ricky) <rickyepoderi at yahoo dot es>
> + * plugin/icedteanp/java/sun/applet/PluginStreamHandler.java
> + (readPair): New function.
> + (handleMessage): Use readPair to incrementally tokenize message, rather
> + than using String.split().
> +
> 2011-06-22 Jiri Vanek <jvanek at redhat.com>
>
> * tests/report-styles/jreport.xsl: part with classes statistics is now collapsable
> diff -r 86abbf8be0b1 NEWS
> --- a/NEWS Thu Jun 23 15:29:45 2011 +0200
> +++ b/NEWS Tue Jul 05 11:12:20 2011 -0400
> @@ -8,6 +8,10 @@
>
> CVE-XXXX-YYYY: http://www.cve.mitre.org/cgi-bin/cvename.cgi?name=XXXX-YYYY
>
> +New in release 1.2 (2011-XX-XX):
> +* Plugin
> + - PR749: sun.applet.PluginStreamHandler#handleMessage(String) really slow
> +
> New in release 1.1 (2011-XX-XX):
> * Security updates
> - S6983554, CVE-2010-4450: Launcher incorrect processing of empty library path entries
> diff -r 86abbf8be0b1 plugin/icedteanp/java/sun/applet/PluginStreamHandler.java
> --- a/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java Thu Jun 23 15:29:45 2011 +0200
> +++ b/plugin/icedteanp/java/sun/applet/PluginStreamHandler.java Tue Jul 05 11:12:20 2011 -0400
> @@ -113,18 +113,58 @@
> listenerThread.start();
> }
>
> + /**
> + * Given a string, reads the first two (space separated) tokens.
> + *
> + * @param message The string to read
> + * @param start The position to start reading at
> + * @param array The array into which the first two tokens are placed
> + * @return Position where the next token starts
> + */
> + private int readPair(String message, int start, String[] array) {
> +
> + int end = start;
> + array[0] = null;
> + array[1] = null;
> +
> + if (message.length() > start) {
> + int firstSpace = message.indexOf(' ', start);
> + if (firstSpace == -1) {
> + array[0] = message.substring(start);
> + end = message.length();
> + } else {
> + array[0] = message.substring(start, firstSpace);
> + if (message.length() > firstSpace + 1) {
> + int secondSpace = message.indexOf(' ', firstSpace + 1);
> + if (secondSpace == -1) {
> + array[1] = message.substring(firstSpace + 1);
> + end = message.length();
> + } else {
> + array[1] = message.substring(firstSpace + 1, secondSpace);
> + end = secondSpace + 1;
> + }
> + }
> + }
> + }
> +
> + PluginDebug.debug("readPair: '", array[0], "' - '", array[1], "' ", end);
> + return end;
> + }
> +
> public void handleMessage(String message) throws PluginException {
>
> - int nextIndex = 0;
> int reference = -1;
> String src = null;
> String[] privileges = null;
> String rest = "";
> + String[] msgComponents = new String[2];
> + int pos = 0;
> + int oldPos = 0;
>
> - String[] msgComponents = message.split(" ");
> -
> - if (msgComponents.length < 2)
> + pos = readPair(message, oldPos, msgComponents);
> + if (msgComponents[0] == null || msgComponents[1] == null) {
> return;
> + }
>
> if (msgComponents[0].startsWith("plugin")) {
> handlePluginMessage(message);
> @@ -134,38 +174,38 @@
> // type and identifier are guaranteed to be there
> String type = msgComponents[0];
> final int identifier = Integer.parseInt(msgComponents[1]);
> - nextIndex = 2;
>
> // reference, src and privileges are optional components,
> // and are guaranteed to be in that order, if they occur
> + oldPos = pos;
> + pos = readPair(message, oldPos, msgComponents);
>
> // is there a reference ?
> - if (msgComponents[nextIndex].equals("reference")) {
> - reference = Integer.parseInt(msgComponents[nextIndex + 1]);
> - nextIndex += 2;
> + if ("reference".equals(msgComponents[0])) {
> + reference = Integer.parseInt(msgComponents[1]);
> + oldPos = pos;
> + pos = readPair(message, oldPos, msgComponents);
> }
>
> // is there a src?
> - if (msgComponents[nextIndex].equals("src")) {
> - src = msgComponents[nextIndex + 1];
> - nextIndex += 2;
> + if ("src".equals(msgComponents[0])) {
> + src = msgComponents[1];
> + oldPos = pos;
> + pos = readPair(message, oldPos, msgComponents);
> }
>
> // is there a privileges?
> - if (msgComponents[nextIndex].equals("privileges")) {
> - String privs = msgComponents[nextIndex + 1];
> + if ("privileges".equals(msgComponents[0])) {
> + String privs = msgComponents[1];
> privileges = privs.split(",");
> - nextIndex += 2;
> + oldPos = pos;
> }
>
> // rest
> - for (int i = nextIndex; i < msgComponents.length; i++) {
> - rest += msgComponents[i];
> - rest += " ";
> + if (message.length() > oldPos) {
> + rest = message.substring(oldPos);
> }
>
> - rest = rest.trim();
> -
> try {
>
> PluginDebug.debug("Breakdown -- type: ", type, " identifier: ", identifier, " reference: ", reference, " src: ", src, " privileges: ", privileges, " rest: \"", rest, "\"");
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list