[icedtea-web] RFC: Patch to fix PR794 (Class-Path element processing)

[Omair Majid]

On 09/28/2011 02:57 PM, Deepak Bhole wrote:
> * Omair Majid<omajid at redhat.com>  [2011-09-27 21:07]:
>> On 09/27/2011 06:07 PM, Deepak Bhole wrote:
>>> Hi,
>>>
>>> Attached patch fixes PR794:
>>> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=794
>>>
>>> ChangeLog:
>>> 2011-09-27  Deepak Bhole<dbhole at redhat.com>
>>>
>>>      PR794: IcedTea-Web does not work if a Web Start app jar has a Class-Path
>>>      element in the manifest.
>>>      * netx/net/sourceforge/jnlp/runtime/CachedJarFileCallback.java
>>>      (retrieve): Blank out the Class-Path elements in manifest.
>>>      * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>>      (activateJars): Only load Class-Path elements if this is an applet. Add a
>>>      security mapping for jars from the Class-Path.
>>>
>>> Okay for HEAD and 1.1?
>>>
>>
>> While the overall idea looks fine to me, I have a few concerns noted
>> inline below.
>>
>>> diff -r 0a1733685325 netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Fri Sep 23 12:14:39 2011 -0400
>>> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Tue Sep 27 18:02:12 2011 -0400
>>> @@ -779,7 +778,15 @@
>>>
>>>                               JarFile jarFile = new JarFile(localFile.getAbsolutePath());
>>>                               Manifest mf = jarFile.getManifest();
>>> -                            classpaths.addAll(getClassPathsFromManifest(mf, jar.getLocation().getPath()));
>>> +
>>> +                            if (file instanceof PluginBridge) {
>>> +                                for (String classpath: getClassPathsFromManifest(mf, jar.getLocation().getPath())) {
>>> +                                    URL codebaseURL = file.getCodeBase();
>>> +                                    jarLocationSecurityMap.put(new URL(codebaseURL.getProtocol() + "://" + codebaseURL.getHost() + classpath), jarSecurity);
>>
>> I have a concern about this line here ^
>>
>> Is this jar being verified before we grant it permissions? It's not
>> obvious from the patch that we are.
>>
>
> Ah, I thought addNewJar was already doing it.
>
> New patch attached. It adds the right permissions and does verification.
>
>> Also, a nitpick: we are ignoring any possible port numbers in
>> codebaseURL. Other code in this class does something along the lines
>> of:
>>
>> new URL(codebaseURL, classpath)
>>
>
> Fixed.
>
> New ChangeLog:
> 2011-09-27  Deepak Bhole<dbhole at redhat.com>
>
>      PR794: IcedTea-Web does not work if a Web Start app jar has a Class-Path
>      element in the manifest.
>      * netx/net/sourceforge/jnlp/runtime/CachedJarFileCallback.java
>      (retrieve): Blank out the Class-Path elements in manifest.
>      * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>      (activateJars): Only load Class-Path elements if this is an applet.
> 	(addNewJar): Add the right permissions for the cached jar file and verify
> 	signatures.
>

Looks good to me!

Cheers,
Omair