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

[Deepak Bhole]

* Deepak Bhole <dbhole at redhat.com> [2011-03-01 17:23]:
> * 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.
> 

Sorry, scratch that. Just came to mind that location won't be null for
systemSource, source will be null. So what is an example where location
is null?

Deepak

> 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();
> > >
> >