[RFC][icedtea-web]: Fix PR909 - The Java applet at http://de.gosupermodel.com/games/wardrobegame.jsp fails

[Saad Mohammad]

On 10/23/2012 05:04 AM, Jiri Vanek wrote:
> 
> Hi!
> 
> Looks much better:)
> 
> There is unit test for this behaviour in
> tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java
> a) ensure it is passing with your fix*
> b) add unit test with @bug which is testing your new behaviour (focused on 909
> simulation)
> 
> * I'm not sure how your patch is behaving when unexpected (encoded/decoded to
> expected decoded/encoded) (lines 52-64 of your patch) state of coding is
> delivered. If your fix pass above test, then it should be ok.
>   As far as I tested it, you are failing for item 1 an 3 in unittest:(
>   For 1 it is clear failure - try eg
> "http://localhost:44321/gg%2Ffgg%15fdgfd%45fdg" where it is  better to see, for
> 2, you can remove this test.
>   The failure (1) is hard to explain. Just from quick check - it looks like url
> and uri are making different encding/decoding. Please, investigate on it.
> 
> Otherwise I have no objection against code and logic.
> 
> Than you for fixing it.
> 
>   J.

Hi Jiri,

Sorry for the late reply.

Attached you'll find the patch with unit-test changes to better work
with PR909 fix. Below is a little explanation of these changes.
I also added a test which validates PR909's case. Let me know your thoughts!

Bug report: <http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=909>
(Basically '=' is being encoded to '%3D' when it shouldn't be)

** NOTES **

After taking a further look into the problem within ResourceTrackerTest.java, I
believe the issue is behind the test and not with URI encoding/decoding. The
reasoning to these failure are because:
      1) URI.encode will encode '+' to '%20', test expects '+' to remain static
         - I think it's fine as they both represent the same value
      2) URI.encode will always decode '%2F' to the decoded value ('/')
         - Problems can arise if the server allows percent encoded slashes

These two particular tests can be removed IMO. But like I mentioned above, there
may be problems if a server configuration allows encoded characters in
their URI.

URI encoding is not a perfect representation of RFC3986 specification, but it
does do a better job than our current implementation (also fixing this bug).
<http://tools.ietf.org/html/rfc3986#section-2.1>

** OTHER NOTES **

Here are the difference I found between URI encoding and our current
implementation.

URI will _encode_ but our current implementation will not:
"
+

URI will _not_encode_ but our current implementation will:
:
;
= (This is what causes PR909)
@
~
%
?

========================================================================
CHANGELOG - BUG FIX
========================================================================
2012-12-18  Saad Mohammad  <smohammad at redhat.com>

       * tests/netx/unit/net/sourceforge/jnlp/cache/ResourceTrackerTest.java:
       Added test and changes to work better with PR909 fix.



-- 
Cheers,
Saad Mohammad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unittest0-3.patch
Type: text/x-patch
Size: 2519 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121218/e25efdad/unittest0-3.patch