[RFC][icedtea-web] Fix for PR855: AppletStub getDocumentBase() doesn't return full URL

[Danesh Dadachanji]

On 18/05/12 05:15 AM, Jiri Vanek wrote:
> On 05/17/2012 11:16 PM, Danesh Dadachanji wrote:
>>
>> On 16/04/12 01:18 PM, Deepak Bhole wrote:
>>> * Danesh Dadachanji<ddadacha at redhat.com> [2012-02-03 15:40]:
>>>> Hi,
>>>>
>>>> This patch is a fix for PR855[1]. The return of getDocumentBase()
>>>> currently doesn't include the HTML file. According to the docs, it
>>>> should return the entire path of the HTML, including the filename.
>>>>
>>>> Attached is a patch that adds this. I've checked all the traces of
>>>> the bytes grabbed from the pipe over on the java side. I believe I
>>>> covered every read and assignment of the buffer/vars setup from the
>>>> buffer and so on. We fortunately only ever used getDocumentBase()'s
>>>> return to grab the host IP or use it as a base URL. The latter code
>>>> grabbed the return and cut off everything after the last '/', which
>>>> back before patch is the entire URL, post patch is the URL excluding
>>>> the file name.
>>>>
>>>> Okay for HEAD? Thoughts on backporting to release branches? It was
>>>> reported against 1.1.3 but this is a pretty big change that
>>>> potentially affects the way users' applications behave.
>>>>
>>>
>>> Looking at the surrounding code, we are taking a URL, splitting it on
>>> "/" and then combining the first length-1 components. This was done to
>>> purposely remove the file name itself.
>>>
>>> However if we want to add the file name back, we should just remove all
>>> of the splitting/combing code rather than appending the last part.
>>>
>>> Have you tried using NPVARIANT_TO_STRING(href).utf8characters directly?
>>>
>>
>> Ah yes I wasn't paying attention to the surrounding code :S
>>
>> How's this look? I'd like to backport this to 1.2 as well. The reporter uses 1.1.3 though so perhaps
>> it should go into 1.1 too? Thoughts?
>>
>> Cheers,
>> Danesh
> Can we get also test for this? :(

Here we go. =) Since there was a long delay in this (sorry!), the original file has changed quite a bit in the last few days, I have 
been waiting for those to go in so I could update mine accordingly. Please note the new changes!

I've added a test for this and a test for checking what codebase should be. I also threw in an extra jnlp_href test because we don't 
have one! 3 tests with one stone. ;)

I also took the opportunity to make some minor typos/mistakes in AppletTest's asserts.

ChangeLog:
+2012-06-04  Danesh Dadachanji  <ddadacha at redhat.com>
+
+	PR855: AppletStub getDocumentBase() doesn't return full URL
+	* plugin/icedteanp/IcedTeaNPPlugin.cc (plugin_get_documentbase):
+	Assign documentbase_copy directly to href's value instead of iterating
+	through the segments to remove the file from the path.
+	* tests/jnlp_tests/simple/AppletTest/srcs/AppletTest.java:
+	Added prints of codebase and document base when initializing.
+	* tests/jnlp_tests/simple/AppletTest/testcases/AppletTestTests.java:
+	(evaluateApplet): New param baseName, represents the HTML file name
+	when calling applet.getDocumentBase().
+	* tests/jnlp_tests/simple/AppletTest/resources/appletJNLPHrefTest.html:
+	New test that runs AppletTest via jnlp_href.
+

I would still like to backport this to 1.2 but it may require me backporting the fixes for PR518 and PR863. I'm unsure if it needs to 
go to 1.1, the reporter said they used 1.1 though. Thoughts?

Cheers,
Danesh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: get-document-base-03.patch
Type: text/x-patch
Size: 8456 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120604/c7892079/get-document-base-03.patch