[rfc] [icedtea-web] fix for PR811

[Pavel Tisnovsky]

Hi Jiri,

the icedtea-web patch (w/o tests):
it think it's ok, but please fix this typo in a comment:

sielnt -> silent

Personally I'd rather to rewrite notNullUrlEquals() method not to use
if-then-else blocks at all (you can directly return the return value of
the expression) but it's just a personal feeling ;-)


the test patch:
I'm not sure if following lines are ok in case of JDK7:

+        System.out.println("connecting NotOnly spaces can kill ěščřž too.jnlp request");
+        System.err.println("connecting NotOnly spaces can kill ěščřž too.jnlp request");


Cheers,
Pavel



Jiri Vanek wrote:
> On 05/17/2012 03:48 PM, Pavel Tisnovsky wrote:
>> Jiri Vanek wrote:
>>> On 05/16/2012 05:00 PM, Pavel Tisnovsky wrote:
> ...
>>>
>>> So You are an voulnteer to review both fix and tests? Thanx a lot! But
>>
>> well I'll try! Below are my comments:
>>
>>> be very carefull. I'm afraid URL of resources is one of basic stones in
>>> netx. I have tried really a lot to figure what everything is affected by
>>> this change, and it looks like all. And there are dark corners in netx;)
>>>
>>>
>>> Thanx a lot again - J.
>>>
> 
> all the indentations and typos should be fixed now
> 
>> +        StringBuilder composed = new StringBuilder("");
>>
>> I'm not 100% sure about following code. At least it is IMHO better to
>> use characters instead of one-characters strings and refactor these
>> magical constants into final fields - PATH_SEPARATOR, QUERY_SEPARATOR
>> etc. etc.
> 
> Done. I have tasted a bit against strange characters and looks fine.
> I have also realized that I forgot to encode query. It is now done.
> 
>>
>> Btw is it really working for all characters - ie. the ones outside of
>> ASCII?
>>
>> +        for (int i = 0; i<  ss.length; i++) {
>> +
>>   }
> 
> 2012-05-18  Jiri Vanek  <jvanek at redhat.com>
> 
>     More tests for Spaces and characters in urls
>     * netx/net/sourceforge/jnlp/cache/CacheLRUWrapper.java: and
>     * netx/net/sourceforge/jnlp/cache/CacheUtil.java: for unit-tests
>     purposes (cacheDir) make to point to tmp dir when no
>     DeploymentConfiguration    exists.
>     * tests/jnlp_tests/signed/Spaces can be everywhere signed/:
>     couple of new test dooing the same as simple "Spaces can be everywhere"
>     but are signed
>     * tests/jnlp_tests/simple/Spaces can be everywhere/: added new
> test-cases
>     and html/jnlp test files to try more combinations of encodable
> characters
>     x launches
>     * tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java:
>     unittest for url encoder behavior
>     * tests/netx/unit/net/sourceforge/jnlp/cache/CacheUtilTest.java:
>     unittest for urlEquals function
>     
> 
> 2012-05-18  Jiri Vanek  <jvanek at redhat.com>
> 
>     Fixed behavior when encoded/characters needed encoding included in url
>     * NEWS: mentioned PR811
>     * netx/net/sourceforge/jnlp/cache/CacheUtil.java: (urlEquals) Enhanced
>     to be able compare encoded/decoded urls correctly.
>     (notNullUrlEquals) new method to separate comparing of individual
> parts of
>     url from null checks
>     * netx/net/sourceforge/jnlp/cache/ResourceTracker.java: (addResource)
>     is now encoding url if needed. (normalizeUrl) new method to encode
> path in
>     url of all except file protocol. (normalizeChunk) New method for
> encoding
>     of atomic piece.
>