[rfc][icedtea-web] Fix regression in SpacesCanBeEveryWhere reproducer

[Jiri Vanek]

On 04/25/2013 03:07 PM, Adam Domurad wrote:
> On 04/25/2013 05:03 AM, Jiri Vanek wrote:
>> On 04/24/2013 09:30 PM, Adam Domurad wrote:
>>> 2013-XX-XX Adam Domurad <adomurad at redhat.com>
>>>
>>> * netx/net/sourceforge/jnlp/cache/ResourceTracker.java
>>> (getCacheFile): Use decodeUrlAsFile instead of toURI().getPath().
>>> * netx/net/sourceforge/jnlp/util/UrlUtils.java
>>> (decodeUrlAsFile): New, tolerates ill-formed URLs.
>>> * tests/netx/unit/net/sourceforge/jnlp/util/UrlUtilsTest.java:
>>> (testDecodeUrlAsFile): Test for (decodeUrlAsFile)
>>>
>>> Sorry this wasn't caught sooner, and thanks to Jiri for pointing it out.
>>> The problem was that archives can't be guaranteed to be encoded URLs.
>>> We instead use an encoding-tolerant function.
>>>
>>> Note that the headache is caused because Java allows essentially invalid URLs to be created (and, on
>>> our side, that we allow them to be created).
>>>
>>> Happy hacking,
>>> -Adam
>>>
>>> fix-paths-with-spaces-regression.patch
>>>
>>>
>>> diff --git a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
>>> --- a/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
>>> +++ b/netx/net/sourceforge/jnlp/cache/ResourceTracker.java
>>> @@ -390,7 +390,7 @@ public class ResourceTracker {
>>> return resource.localFile;
>>>
>>> if (location.getProtocol().equalsIgnoreCase("file")) {
>>> - File file = new File(location.toURI().getPath());
>>> + File file = UrlUtils.decodeUrlAsFile(location);
>>> if (file.exists())
>>> return file;
>>> }
>>> @@ -401,9 +401,6 @@ public class ResourceTracker {
>>> ex.printStackTrace();
>>>
>>> return null; // need an error exception to throw
>>> - } catch (URISyntaxException e) {
>>> - e.printStackTrace();
>>> - return null;
>>> }
>>
>>
>> Just ensuring about the removal of the exception:
>
> Which was added in a patch you reviewed a few days ago :-D
>
>>
>> When decodeUrlQuietly "fails" it prints out exception (not throw) and return original url.
>> So no NPE is thrown during the getCacheFile method propcessing.
>> However returning of null was expected when url was wrong. However now "strange" url is returned....
>
> This exception catch didn't exist before the last batch of changes. It's just noise introduced because of checked exceptions (Oh how I *hate* them :-).
> I don't know what you mean by 'strange' URLs. In the case decodeUrlQuietly fails the code will be exactly like it was before (before the last batch of commits anyway), except print more exceptions. This is because it is a no-op when it fails, which renders it the same as the code that was there before the last batch of changes.

As hoped, go on and push .

>
>>
>> Checking the usage of getCacheFile it is probably more usfeul.. and we will recieve "some strange exception" instead of NPE...
>>
>> J.
>>
>> After this explained, looks good to go.
>>
>>> }
>>> [..snipped..]
>
> Thanks again for catching it. Very sorry for the breakage.
> -Adam