[RFC][icedtea-web] Fix handling absolute paths passed into jnlp_href's value.

[Danesh Dadachanji]

On 20/04/12 06:06 AM, Jiri Vanek wrote:
> On 03/12/2012 10:25 PM, Danesh Dadachanji wrote:
>> Hi,
>>
>> The Knuddels test applet[1] uses an absolute path to the JNLP when using jnlp_href. Right now we
>> assume it's a relative path and take the document base (i.e. the URL of the dir in which HTML file
>> is located) concatenated with the value of jnlp_href for the new URL.
>>
>> The attached patch updates PluginBridge to handle absolute paths to the JNLP. I simply used the
>> document base as the context to create a new URL. Based on the javadoc for this, if jnlp_href's
>> value is not a properly formed URL, it will default to using the document base's URL as the base URL
>> and append the relative part (value of jnlp_href) to the end of it.
>>
>> The URL constructor also handles relative paths so if codebase="../example.jnlp", the constructor
>> will act accordingly and remove a dir if possible.
>>
>> +2012-03-09 Danesh Dadachanji <ddadacha at redhat.com>
>> +
>> + Fix to handle absolute paths passed into jnlp_href's value.
>> + * netx/net/sourceforge/jnlp/PluginBridge.java
>> + (PluginBridge): Use the document base as a context for the URL
>> + being initialized by jnlp_href's value. This will automatically handle
>> + relative and absolute URL paths.
>> +
>>
>> Any comments? Can I push this to HEAD?
>>
>> NOTE: [1] still won't work because it has invalid XML. However you should see that it can now find
>> the JNLP at the correct URL as opposed to
>>
>> java.io.IOException: http://chat.knuddels.de/http://www.knuddels.de/applet.jnlp[...]
>>
>> Regards,
>> Danesh
>>
>> [1] www.knuddels.de:8080/index.html?v=90aez&c=7
>
> This looks ok to me. Can you add unit tests please?
>
> (feel free to some refactoring if necessary for testing purposes)
>

Attached a new patch with the refactored code and a unit test.

I added a static method that just wraps around the new URL assignment. I couldn't figure out how to test PluginBridge directly without 
having it open an actual JNLP file. :/ I made it static to make the unit test cleaner (did not have to deal with creating a new 
PluginBridge var etc) since it is quite the simple method and really only serves for unit testing purposes. If others prefer, I can 
make it non-static.

ChangeLog:
+2012-05-30  Danesh Dadachanji  <ddadacha at redhat.com>
+
+	Fix to handle absolute paths passed into jnlp_href's value.
+	* netx/net/sourceforge/jnlp/PluginBridge.java
+	(PluginBridge): jnlp_href is examined for an absolute or relative path.
+	(getEvaluatedJNLPHref): New static method to construct URL to the JNLP.
+	Examines the jnlp_href value for an absolute, uses codebase
+	as a context if it is relative.
+	* tests/netx/unit/net/sourceforge/jnlp/PluginBridgeTest.java:
+	New class to unit test getEvaluatedJNLPHref.
+
+


Thanks for the review!

Cheers,
Danesh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: absolute-url-jnlp_href-02.patch
Type: text/x-patch
Size: 2735 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120530/7ea61013/absolute-url-jnlp_href-02.patch