[icedtea-web] RFC: PR898: signed applications with big jnlp-file doesn't start
Jiri Vanek
jvanek at redhat.com
Thu Mar 22 04:56:38 PDT 2012
On 03/22/2012 12:37 PM, Lars Herschke wrote:
> Hi,
>
>> those two toString() should be definietly called with encoding. Although it is not deprecated (as methods without encoding are in most of other classes), it can cause mess.
>> http://docs.oracle.com/javase/6/docs/api/java/io/ByteArrayOutputStream.html#toString%28java.lang.String%29
>
> InputStreamReader has also doing a conversion into characters.
Yes - and set up an encoding is strongly recommended.
>
>> One more idea I have to this patch is in Parser (net/sourceforge/jnlp/Parser.java) is also used Piped mechanism. Isn't worthy to fix it too?
>
> Parser.java use threads, so there is no patch necessary.
Did it? ok then.
>
>
>
> Jiri Vanek schrieb:
>> On 03/21/2012 03:44 PM, Omair Majid wrote:
>>> Hi,
>>>
>>> The attached patch fixes PR898: signed applications with big jnlp-file
>>> doesn't start. We were using piped input and output streams without
>>> threads which would cause them to block and javaws to hang. Switching to
>>> ByteArrayOuputStream/StringReader takes care of this.
>>>
>>> Okay for HEAD and 1.2?
>> Hi Omair!
>>
>> Ty for patch, I think that test can go in immediately (even separately).
>> For patch itself I have few comments inline
>>
>>>
>>> Thanks,
>>> Omair
>>>
>>>
>>> pr898-01.patch
>>>
>>>
>>> diff --git a/ChangeLog b/ChangeLog
>>> --- a/ChangeLog
>>> +++ b/ChangeLog
>>> @@ -1,3 +1,13 @@
>>> +2012-03-21 Omair Majid<omajid at redhat.com>
>>> +
>>> + * tests/netx/unit/net/sourceforge/jnlp/JNLPMatcherTest.java
>>> + (testIsMatchDoesNotHangOnLargeData): New method.
>>> +
>>> +2012-03-21 Lars Herschke<lhersch at dssgmbh.de>
>>> +
>>> + * netx/net/sourceforge/jnlp/JNLPMatcher.java (JNLPMatcher):
>>> Handle large
>>
>> Little bit more like replacing pipes by ArrayStreams can be better, but
>> I do not insists as You are just pushing other's changeset.
>>
>>> + files correctly.
>>> +
>>> 2012-03-19 Danesh Dadachanji<ddadacha at redhat.com>
>>>
>>> Fix failing unit test missing title/vendor tags in the JNLP stream.
>>> diff --git a/netx/net/sourceforge/jnlp/JNLPMatcher.java
>>> b/netx/net/sourceforge/jnlp/JNLPMatcher.java
>>> --- a/netx/net/sourceforge/jnlp/JNLPMatcher.java
>>> +++ b/netx/net/sourceforge/jnlp/JNLPMatcher.java
>>> @@ -38,11 +38,12 @@
>>> package net.sourceforge.jnlp;
>>>
>>> import java.util.List;
>>> +import java.io.ByteArrayOutputStream;
>>> import java.io.InputStream;
>>> import java.io.InputStreamReader;
>>> import java.io.OutputStream;
>>> -import java.io.PipedInputStream;
>>> -import java.io.PipedOutputStream;
>>> +import java.io.Reader;
>>> +import java.io.StringReader;
>>> import java.util.Arrays;
>>> import java.util.Collections;
>>> import java.util.LinkedList;
>>> @@ -75,7 +76,7 @@
>>> * if IOException, XMLParseException is thrown
>>> during parsing;
>>> * Or launchJNLP/appTemplate is null
>>> */
>>> - public JNLPMatcher(InputStreamReader appTemplate,
>>> InputStreamReader launchJNLP,
>>> + public JNLPMatcher(Reader appTemplate, Reader launchJNLP,
>>> boolean isTemplate) throws JNLPMatcherException {
>>>
>>> if (appTemplate == null&& launchJNLP == null)
>>> @@ -87,29 +88,25 @@
>>> throw new JNLPMatcherException("Launching JNLP file is
>>> null.");
>>>
>>> //Declare variables for signed JNLP file
>>> - PipedInputStream pinTemplate= null;
>>> - PipedOutputStream poutTemplate= null;
>>> + ByteArrayOutputStream poutTemplate= null;
>>>
>>> //Declare variables for launching JNLP file
>>> - PipedInputStream pinJNLPFile = null;
>>> - PipedOutputStream poutJNLPFile = null;
>>> + ByteArrayOutputStream poutJNLPFile = null;
>>>
>>> try {
>>> XMLElement appTemplateXML = new XMLElement();
>>> XMLElement launchJNLPXML = new XMLElement();
>>>
>>> // Remove the comments and CDATA from the JNLP file
>>> - pinTemplate = new PipedInputStream();
>>> - poutTemplate = new PipedOutputStream(pinTemplate);
>>> + poutTemplate = new ByteArrayOutputStream();
>>> appTemplateXML.sanitizeInput(appTemplate, poutTemplate);
>>>
>>> - pinJNLPFile = new PipedInputStream();
>>> - poutJNLPFile = new PipedOutputStream(pinJNLPFile);
>>> + poutJNLPFile = new ByteArrayOutputStream();
>>> launchJNLPXML.sanitizeInput(launchJNLP, poutJNLPFile);
>>>
>>> // Parse both files
>>> - appTemplateXML.parseFromReader(new
>>> InputStreamReader(pinTemplate));
>>> - launchJNLPXML.parseFromReader(new
>>> InputStreamReader(pinJNLPFile));
>>> + appTemplateXML.parseFromReader(new
>>> StringReader(poutTemplate.toString()));
>>
>> those two toString() should be definietly called with encoding. Although
>> it is not deprecated (as methods without encoding are in most of other
>> classes), it can cause mess.
>> http://docs.oracle.com/javase/6/docs/api/java/io/ByteArrayOutputStream.html#toString%28java.lang.String%29
>>
>>
>> There is small pitfall anyway - you should use the encoding defined in
>> jnlp's xml file instead of any "hardcoded". (and I think that with this
>> new approach sting<=> byte[] without pipes you can easily do this). I'm
>> afraid most of jnlp files have been written on windows machines anyway:)
>>
>> If you are ok now with default utf-8 encoding, I'm for, and if you will
>> consider the encoding reading and then re-encode if necessary, then it
>> can be as separate patch.
>>
>>> + launchJNLPXML.parseFromReader(new
>>> StringReader(poutJNLPFile.toString()));
>>>
>>> // Initialize parent nodes
>>> this.appTemplateNode = new Node(appTemplateXML);
>>> @@ -122,10 +119,8 @@
>>> e);
>>> } finally {
>>> // Close all stream
>>> - closeInputStream(pinTemplate);
>>> closeOutputStream(poutTemplate);
>>>
>>> - closeInputStream(pinJNLPFile);
>>> closeOutputStream(poutJNLPFile);
>>>
>>> }
>>
>> One more idea I have to this patch is in Parser
>> (net/sourceforge/jnlp/Parser.java) is also used Piped mechanism. Isn't
>> worthy to fix it too? And there is also some wired encoing
>> recognition... At least look at it (as I was gazing to it quite confused).
>>
>> I was also thinking that get rid of buffering can cause unnecessary
>> requirements but at he end I think there is just minor increase if any.
>> Please correct me If I'm judging it wrongly.
>>
>>
>>> diff --git a/tests/netx/unit/net/sourceforge/jnlp/JNLPMatcherTest.java
>>> b/tests/netx/unit/net/sourceforge/jnlp/JNLPMatcherTest.java
>>> --- a/tests/netx/unit/net/sourceforge/jnlp/JNLPMatcherTest.java
>>> +++ b/tests/netx/unit/net/sourceforge/jnlp/JNLPMatcherTest.java
>>> @@ -37,12 +37,13 @@
>>>
>>> package net.sourceforge.jnlp;
>>>
>>> -import static org.junit.Assert.fail;
>>> -
>>> import java.io.IOException;
>>> import java.io.InputStream;
>>> import java.io.InputStreamReader;
>>> -import junit.framework.Assert;
>>> +import java.io.StringReader;
>>> +import java.util.Random;
>>> +
>>> +import org.junit.Assert;
>>> import org.junit.Test;
>>>
>>> public class JNLPMatcherTest {
>>> @@ -461,4 +462,36 @@
>>> fileReader.close();
>>> launchReader.close();
>>> }
>>> +
>>> + @Test (timeout=1000 /*ms*/)
>>> + public void testIsMatchDoesNotHangOnLargeData() throws
>>> JNLPMatcherException {
>>> + /* construct an alphabet containing characters 'a' to 'z' */
>>> + final int ALPHABET_SIZE = 26;
>>> + char[] alphabet = new char[ALPHABET_SIZE];
>>> + for (int i = 0; i< ALPHABET_SIZE; i++) {
>>> + alphabet[i] = (char)('a' + i);
>>> + }
>>> + /* generate a long but random string using the alphabet */
>>> + final Random r = new Random();
>>> + final int STRING_SIZE = 1024 * 1024; // 1 MB
>>> + StringBuilder descriptionBuilder = new
>>> StringBuilder(STRING_SIZE);
>>> + for (int i = 0; i< STRING_SIZE; i++) {
>>> +
>>> descriptionBuilder.append(alphabet[r.nextInt(ALPHABET_SIZE)]);
>>> + }
>>> + String longDescription = descriptionBuilder.toString();
>>> +
>>> + String file =
>>> + "<jnlp>\n" +
>>> + "<information>\n" +
>>> + "<title>JNLPMatcher hanges on large file
>>> size</title>\n" +
>>> + "<vendor>IcedTea</vendor>\n" +
>>> + "<description>" + longDescription +"</description>\n" +
>>> + "</information>\n" +
>>> + "</jnlp>\n";
>>> +
>>> + StringReader reader1 = new StringReader(file);
>>> + StringReader reader2 = new StringReader(file);
>>> + JNLPMatcher matcher = new JNLPMatcher(reader1, reader2, false);
>>> + Assert.assertTrue(matcher.isMatch());
>>> + }
>>> }
>>>
>>
>>
>> Best regards and sorry if I read the patch wrongly
>> J.
>>
More information about the distro-pkg-dev
mailing list