Fixing design flaw in SecurityController:

The new SecurityController in its current form does not allow to define more then one generated class class in the same class loader effectively preventing to use optimizer which needs to define classes that refer each other and should be defined in the same loader.

To fix this I replaced the defineClass method in SecurityController by
    public GeneratedClassLoader createClassLoader(Object securityDomain);
which returns instance of the new GeneratedClassLoader interface which can be used to define several classes. I also made DefiningClassLoader to implement this interface to simplify code in JavaAdapter.java and optimizer/Codegen.java.

js/rhino/src/org/mozilla/classfile/DefiningClassLoader.java

@@ -40,12 +40,16 @@ package org.mozilla.classfile;
 import java.lang.reflect.Method;
 import java.lang.reflect.InvocationTargetException;
 
+import org.mozilla.javascript.GeneratedClassLoader;
+
 /**
  * Load generated classes.
  *
  * @author Norris Boyd
  */
-public class DefiningClassLoader extends ClassLoader {
+public class DefiningClassLoader extends ClassLoader
+    implements GeneratedClassLoader
+{
 
     public static ClassLoader getContextClassLoader() {
         try {
@@ -62,10 +66,14 @@ public class DefiningClassLoader extends ClassLoader {
         return DefiningClassLoader.class.getClassLoader();
     }
 
-    public Class defineClass(String name, byte data[]) {
+    public Class defineClass(String name, byte[] data) {
         return super.defineClass(name, data, 0, data.length);
     }
 
+    public void linkClass(Class cl) {
+        resolveClass(cl);
+    }
+
     public Class loadClass(String name, boolean resolve)
         throws ClassNotFoundException
     {

js/rhino/src/org/mozilla/javascript/GeneratedClassLoader.java

@@ -0,0 +1,63 @@
+/* -*- Mode: java; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ *
+ * The contents of this file are subject to the Netscape Public
+ * License Version 1.1 (the "License"); you may not use this file
+ * except in compliance with the License. You may obtain a copy of
+ * the License at http://www.mozilla.org/NPL/
+ *
+ * Software distributed under the License is distributed on an "AS
+ * IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ * implied. See the License for the specific language governing
+ * rights and limitations under the License.
+ *
+ * The Original Code is Rhino code, released
+ * May 6, 1999.
+ *
+ * The Initial Developer of the Original Code is Netscape
+ * Communications Corporation.  Portions created by Netscape are
+ * Copyright (C) 1997-1999 Netscape Communications Corporation. All
+ * Rights Reserved.
+ *
+ * Contributor(s):
+ * Igor Bukanov
+ *
+ * Alternatively, the contents of this file may be used under the
+ * terms of the GNU Public License (the "GPL"), in which case the
+ * provisions of the GPL are applicable instead of those above.
+ * If you wish to allow use of your version of this file only
+ * under the terms of the GPL and not to allow others to use your
+ * version of this file under the NPL, indicate your decision by
+ * deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL.  If you do not delete
+ * the provisions above, a recipient may use your version of this
+ * file under either the NPL or the GPL.
+ */
+
+// API class
+
+package org.mozilla.javascript;
+
+/**
+ * Interface to define classes from generated byte code.
+ */
+public interface GeneratedClassLoader {
+
+    /**
+     * Define a new Java class.
+     * Classes created via this method should have the same class loader.
+     *
+     * @param name fully qualified class name
+     * @param data class byte code
+     * @return new class object
+     */
+    public Class defineClass(String name, byte[] data);
+
+    /**
+     * Link the given class.
+     *
+     * @param cl Class instance returned from the previous call to
+     *        {@link #defineClass(String, byte[])}
+     * @see java.lang.ClassLoader#resolveClass(Class)
+     */
+    public void linkClass(Class cl);
+}

js/rhino/src/org/mozilla/javascript/JavaAdapter.java

@@ -334,16 +334,17 @@ public class JavaAdapter extends ScriptableObject {
             }
         }
 
+        GeneratedClassLoader loader;
         SecurityController sc = cx.getSecurityController();
-        if (sc != null) {
+        if (sc == null) {
+            loader = new DefiningClassLoader();
+        } else {
             Object securityDomain = sc.getDynamicSecurityDomain(null);
-            Class result = sc.defineClass(adapterName, bytes, securityDomain);
-            if (result != null)
-                return result;
+            loader = sc.createClassLoader(securityDomain);
         }
-        DefiningClassLoader classLoader = new DefiningClassLoader();
-        classLoader.defineClass(adapterName, bytes);
-        return classLoader.loadClass(adapterName, true);
+        Class result = loader.defineClass(adapterName, bytes);
+        loader.linkClass(result);
+        return result;
     }
 
     /**

js/rhino/src/org/mozilla/javascript/SecurityController.java

@@ -65,23 +65,13 @@ package org.mozilla.javascript;
 public abstract class SecurityController {
 
     /**
-     * Define and load a Java class.
-     * <p>
-     * In embeddings that care about security, the securityDomain
-     * must be associated with the defined class such that a call to
-     * <code>getSecurityDomain</code> with that class will return this security
-     * context.
-     * <p>
-     * @param name the name of the class
-     * @param data the bytecode of the class
+     * Get class loader-like object that can be used
+     * to define classes with the given security context.
      * @param securityDomain some object specifying the security
-     *        context of the code that is defining this class.
-     *        Embeddings that don't care about security may allow
-     *        null here. This value propagated from the values passed
-     *        into methods of Context that evaluate scripts.
+     *        context of the code that is defined by the returned class loader.
      */
-    public abstract Class defineClass(String name, byte[] data,
-                                      Object securityDomain);
+    public abstract GeneratedClassLoader
+    createClassLoader(Object securityDomain);
 
     /**
      * Get dynamic security domain that allows an action only if it is allowed

js/rhino/src/org/mozilla/javascript/optimizer/Codegen.java

@@ -80,9 +80,11 @@ public class Codegen extends Interpreter {
 
         Exception e = null;
         Class result = null;
-        DefiningClassLoader classLoader = null;
+        GeneratedClassLoader loader;
         if (securityController == null) {
-            classLoader = new DefiningClassLoader();
+            loader = new DefiningClassLoader();
+        } else {
+            loader = securityController.createClassLoader(securityDomain);
         }
         nameHelper.reset();
 
@@ -98,39 +100,25 @@ public class Codegen extends Interpreter {
                 String name = (String) names.get(i);
                 byte[] classFile = (byte[]) classFiles.get(i);
                 boolean isTopLevel = name.equals(generatedName);
-
                 try {
                     if (repository.storeClass(name, classFile, isTopLevel)) {
-                        Class clazz = null;
-                        if (securityController != null) {
-                            clazz = securityController.
-                                defineClass(name, classFile, securityDomain);
-                            if (clazz == null) {
-                                throw new NullPointerException
-                                    ("SecurityController.defineClass"
-                                     +" may not return null");
-                            }
-                        } else {
-                            clazz = classLoader.defineClass(name, classFile);
-                            ClassLoader loader = clazz.getClassLoader();
-                            clazz = loader.loadClass(name);
+                        Class cl = loader.defineClass(name, classFile);
+                        if (isTopLevel) {
+                            result = cl;
                         }
-                        if (isTopLevel)
-                            result = clazz;
                     }
                 } catch (ClassFormatError ex) {
                     throw new RuntimeException(ex.toString());
-                } catch (ClassNotFoundException ex) {
-                    throw new RuntimeException(ex.toString());
                 } catch (IOException iox) {
                     throw WrappedException.wrapException(iox);
                 }
             }
-        }
-        catch (SecurityException x) {
+            if (result != null) {
+                loader.linkClass(result);
+            }
+        } catch (SecurityException x) {
             e = x;
-        }
-        catch (IllegalArgumentException x) {
+        } catch (IllegalArgumentException x) {
             e = x;
         }
         if (e != null)

js/rhino/toolsrc/org/mozilla/javascript/tools/shell/JavaPolicySecurity.java

@@ -48,10 +48,22 @@ public class JavaPolicySecurity extends SecurityProxy
 {
 
     private static class Loader extends ClassLoader
+        implements GeneratedClassLoader
     {
-        Class defineClass(String name, byte[] data, ProtectionDomain domain) {
+        private ProtectionDomain domain;
+
+        Loader(ClassLoader parent, ProtectionDomain domain) {
+            super(parent != null ? parent : getSystemClassLoader());
+            this.domain = domain;
+        }
+
+        public Class defineClass(String name, byte[] data) {
             return super.defineClass(name, data, 0, data.length, domain);
         }
+
+        public void linkClass(Class cl) {
+            resolveClass(cl);
+        }
     }
 
     private static class ContextPermissions extends PermissionCollection
@@ -134,7 +146,7 @@ public class JavaPolicySecurity extends SecurityProxy
         URL urlObj;
         try {
             urlObj = new URL(url);
-        }catch (MalformedURLException ex) {
+        } catch (MalformedURLException ex) {
             // Assume as Main.processFileSecure it is file, need to build its
             // URL
             String curDir = System.getProperty("user.dir");
@@ -145,7 +157,7 @@ public class JavaPolicySecurity extends SecurityProxy
             try {
                 URL curDirURL = new URL("file:"+curDir);
                 urlObj = new URL(curDirURL, url);
-            }catch (MalformedURLException ex2) {
+            } catch (MalformedURLException ex2) {
                 throw new RuntimeException
                     ("Can not construct file URL for '"+url+"':"
                      +ex2.getMessage());
@@ -160,15 +172,9 @@ public class JavaPolicySecurity extends SecurityProxy
         return new ProtectionDomain(cs, pc);
     }
 
-    void doPrivileged(final Runnable code) {
-    }
-
-    public Class defineClass(String name, byte[] data,
-                             Object securityDomain)
-    {
+    public GeneratedClassLoader createClassLoader(Object securityDomain) {
         ProtectionDomain domain = (ProtectionDomain)securityDomain;
-        Loader loader = new Loader();
-        return loader.defineClass(name, data, domain);
+        return new Loader(getClass().getClassLoader(), domain);
     }
 
     public Object getDynamicSecurityDomain(Object securityDomain)