[RFC] PR858

[Thomas Meyer]

Hello,

looking at bug 858 (e.g.
http://www.chemaxon.com/marvin-archive/5.8.0/marvin/examples/applets/view/embed.html ) icedtea-web gets an NPE:

java.lang.NullPointerException
    at
net.sourceforge.jnlp.runtime.JNLPClassLoader.getPermissions(JNLPClassLoader.java:603)
    at
java.security.SecureClassLoader.getProtectionDomain(SecureClassLoader.java:210)
    at
java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
    at java.net.URLClassLoader.defineClass(URLClassLoader.java:277)
    at java.net.URLClassLoader.access$000(URLClassLoader.java:73)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:212)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:205)
    at
net.sourceforge.jnlp.runtime.JNLPClassLoader.findClass(JNLPClassLoader.java:1187)
    at
net.sourceforge.jnlp.runtime.JNLPClassLoader.loadClassExt(JNLPClassLoader.java:1214)
    at
net.sourceforge.jnlp.runtime.JNLPClassLoader.loadClass(JNLPClassLoader.java:1029)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:186)
    at chemaxon.marvin.applet.AppletLaunch.run(Unknown Source)
    at java.lang.Thread.run(Thread.java:679)

This is because of:

1.) the JNLPClassLoader constructor will only verify and add the jar
"appletlauncher.jar" (only this jar will get a SecurityDesc entry), but
will use the classpath defined in the manifest file, which contains all
other "marvin" jars.

2.) the AppletLaunch class will run next and tries to load another class
from a jar in the classpath. this jar was not jet verified, i.e. has no
SecurtiyDesc entry in the JNLPClassLoader.

3.) the method JNLPClassLoader.loadClass(String name) has this code that
looks like it should add jar files not yet verified, but this code will
(as far as I understand) never be executed reached:

        // search this and the extension loaders
        if (result == null) {
            try {
                result = loadClassExt(name);
            } catch (ClassNotFoundException cnfe) {
                // Not found in external loader either
-> We never get here!
                // Look in 'Class-Path' as specified in the manifest file
                try {
                    for (String classpath: classpaths) {
                        JARDesc desc;
                        try {
                            URL jarUrl = new URL(file.getCodeBase(), classpath);
                            desc = new JARDesc(jarUrl, null, null, false, true, false, true);
                        } catch (MalformedURLException mfe) {
                            throw new ClassNotFoundException(name, mfe);
                        }
                        addNewJar(desc);
                    }

4.) this code is never reached because loadClassExt will call
findClass(). findClass() detects that the current loader is "this"
loader and call the super.findClass(). the super class of
JNLPClassLoader is URLClassLoader which happily loads the jar from the
classpath and will end up calling JNLPClassLoader.getPermission(), which
has not yet a SecurityDesc entry of this jar! As you can see in above
stack trace.

Result:
-------

1.) I guess above code that tries to add the jars from the classpath in
loadClass() will never be reached, and may should get removed.

Opinions please!

2.) see below patch with adds every new jar not yet verified in the
getPermissions() method. This fixes the problem and makes marvin-sketch
work. what makes me wonder is, that no security dialogue is shown, so
that the user should confirm the extra permissions for this applet. on
mac os x and the sun plugin I need to confirm something in an extra
dialogue box ...

kind regards
thomas

diff -r d2aff3800f4f netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Thu Mar 08 15:54:39 2012 +0100
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Sat Mar 10 10:22:01 2012 +0100
@@ -905,13 +905,25 @@
 
             // set default perms
             PermissionCollection permissions = security.getSandBoxPermissions();
+            URL csUrl = cs.getLocation();
+            SecurityDesc sd = getCodeSourceSecurity(csUrl);
+            
+            if(sd == null) {
+                // we don't know this jar yet!
+                
+                // add security context for this jar
+                JARDesc desc = new JARDesc(csUrl, null, null, false, true, false, true);
+                addNewJar(desc);
+                sd = getCodeSourceSecurity(csUrl);
+
+            } 
 
             // If more than default is needed:
             // 1. Code must be signed
             // 2. ALL or J2EE permissions must be requested (note: plugin requests ALL automatically)
-            if (cs.getCodeSigners() != null &&
-                    (getCodeSourceSecurity(cs.getLocation()).getSecurityType().equals(SecurityDesc.ALL_PERMISSIONS) ||
-                     getCodeSourceSecurity(cs.getLocation()).getSecurityType().equals(SecurityDesc.J2EE_PERMISSIONS))) {
+            if (cs.getCodeSigners() != null && sd != null &&
+                    (sd.getSecurityType().equals(SecurityDesc.ALL_PERMISSIONS) ||
+                     sd.getSecurityType().equals(SecurityDesc.J2EE_PERMISSIONS))) {
 
                 permissions = getCodeSourceSecurity(cs.getLocation()).getPermissions(cs);