[icedtea-web] RFC: check for nulls in JNLPPolicy.isSystemJar

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-03-01 17:06]:
> On 03/01/2011 04:48 PM, Deepak Bhole wrote:
> >* Omair Majid<omajid at redhat.com>  [2011-03-01 16:41]:
> >>Hi,
> >>
> >>The attached patch adds a null check in JNLPPolicy.isSystemJar. It's
> >>needed as PluginAppletSecurityContext.getAccessControlContext
> >>creates CodeSources with null locations.
> >>
> >
> >Why is this needed? The function is only called from getPermissions()
> >which does a check on source prior to calling:
> >
> >         if (source.equals(systemSource) || source.equals(shellSource))
> >             return getAllPermissions();
> >
> 
> Actually, the code is more like this:
> 
>     public PermissionCollection getPermissions(CodeSource source) {
>         if (source.equals(systemSource) || source.equals(shellSource))
>             return getAllPermissions();
> 
>         if (isSystemJar(source)) {
>             return getAllPermissions();
>         }
> 

Yep, I know. Which means source is (in some way) looked at before
calling isSystemJar.

> If source is null (or, more importantly, if source.location is null)
> isSystemJar may still be called. Without this patch, isSystemJar
> will throw a NullPointerException instead of returning false.
> 

Well source can't be null else .equals will throw an NPE. As for
source.getLocation.. systemSource is derived as:

systemSource = Policy.class.getProtectionDomain().getCodeSource()

which will have location == null

So in theory there should never be a case where isSystemJar is called
will null source or where source.getLocation is null.

Deepak

> >>ChangeLog:
> >>2011-03-01  Omair Majid<omajid at redhat.com>
> >>
> >>     * netx/net/sourceforge/jnlp/runtime/JNLPPolicy.java (isSystemJar):
> >>     Check for nulls.
> >>
> >>Okay to commit?
> >>
> >>Cheers,
> >>Omair
> >
> >>diff -r c4b91b61f88e netx/net/sourceforge/jnlp/runtime/JNLPPolicy.java
> >>--- a/netx/net/sourceforge/jnlp/runtime/JNLPPolicy.java	Mon Feb 28 17:29:31 2011 -0500
> >>+++ b/netx/net/sourceforge/jnlp/runtime/JNLPPolicy.java	Tue Mar 01 16:31:13 2011 -0500
> >>@@ -137,6 +137,10 @@
> >>       * it's part of the JRE.
> >>       */
> >>      private boolean isSystemJar(CodeSource source) {
> >>+        if (source == null || source.getLocation() == null) {
> >>+            return false;
> >>+        }
> >>+
> >>          // anything in JRE/lib/ext is a system jar and has full permissions
> >>          String sourceProtocol = source.getLocation().getProtocol();
> >>          String sourcePath = source.getLocation().getPath();
> >
>