script performant multi-run support

Author: akostadinov

src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovySandbox.java

@@ -126,16 +126,14 @@ public void run() {
     /**
      * Runs a script in the sandbox.
      * You must have used {@link #createSecureCompilerConfiguration} to prepare the Groovy shell.
+     * Before running a sandboxed script, always {@link org.kohsuke.groovy.sandbox.GroovyInterceptor#register register} and after that make sure to {@link org.kohsuke.groovy.sandbox.GroovyInterceptor#unregister unregister}
      * @param script a script ready to {@link Script#run}, created for example by {@link GroovyShell#parse(String)}
      * @param whitelist the whitelist to use, such as {@link Whitelist#all()}
      * @return the value produced by the script, if any
      * @throws RejectedAccessException in case an attempted call was not whitelisted
      */
     public static Object run(@Nonnull Script script, @Nonnull final Whitelist whitelist) throws RejectedAccessException {
-        Whitelist wrapperWhitelist = new ProxyWhitelist(
-                new ClassLoaderWhitelist(script.getClass().getClassLoader()),
-                whitelist);
-        GroovyInterceptor sandbox = new SandboxInterceptor(wrapperWhitelist);
+        GroovyInterceptor sandbox = createSandbox(script, whitelist);
         sandbox.register();
         try {
             return script.run();
@@ -144,6 +142,20 @@ public static Object run(@Nonnull Script script, @Nonnull final Whitelist whitel
         }
     }
 
+    /**
+     * Prepares a sandbox for a script.
+     * You must have used {@link #createSecureCompilerConfiguration} to prepare the Groovy shell.
+     * @param script a script ready to {@link Script#run}, created for example by {@link GroovyShell#parse(String)}
+     * @param whitelist the whitelist to use, such as {@link Whitelist#all()}
+     * @return the sandbox for running this script
+     */
+    public static GroovyInterceptor createSandbox(@Nonnull Script script, @Nonnull final Whitelist whitelist) {
+        Whitelist wrapperWhitelist = new ProxyWhitelist(
+                new ClassLoaderWhitelist(script.getClass().getClassLoader()),
+                whitelist);
+        return new SandboxInterceptor(wrapperWhitelist);
+    }
+
     private GroovySandbox() {}
 
 }

src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java

@@ -28,6 +28,7 @@
 import groovy.lang.Binding;
 import groovy.lang.GroovyClassLoader;
 import groovy.lang.GroovyShell;
+import groovy.lang.Script;
 import hudson.Extension;
 import hudson.PluginManager;
 import hudson.model.AbstractDescribableImpl;
@@ -36,6 +37,8 @@
 import hudson.util.FormValidation;
 
 import java.beans.Introspector;
+import java.io.IOException;
+import java.io.Closeable;
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
@@ -68,6 +71,7 @@
 import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedClasspathException;
 import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException;
 import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
+import org.kohsuke.groovy.sandbox.GroovyInterceptor;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.Stapler;
@@ -277,6 +281,17 @@ private static void cleanUpObjectStreamClassCaches(@Nonnull Class<?> clazz) thro
         }
     }
 
+
+    /**
+     * Prepares the Groovy script to be executed.
+     * @param loader see parameter explanation under {@link #evaluate(ClassLoader, Binding)}
+     * @param binding see parameter explanation under {@link #evaluate(ClassLoader, Binding)}
+     * @return the prepared script
+     */
+    public PreparedScript prepare(ClassLoader loader, Binding binding) throws IllegalAccessException, IOException {
+      return new PreparedScript(loader, binding);
+    }
+
     /**
      * Runs the Groovy script, using the sandbox if so configured.
      * @param loader a class loader for constructing the shell, such as {@link PluginManager#uberClassLoader} (will be augmented by {@link #getClasspath} if nonempty)
@@ -287,63 +302,96 @@ private static void cleanUpObjectStreamClassCaches(@Nonnull Class<?> clazz) thro
      * @throws UnapprovedUsageException in case of a non-sandbox issue
      * @throws UnapprovedClasspathException in case some unapproved classpath entries were requested
      */
-    @SuppressFBWarnings(value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "Managed by GroovyShell.")
     public Object evaluate(ClassLoader loader, Binding binding) throws Exception {
-        if (!calledConfiguring) {
-            throw new IllegalStateException("you need to call configuring or a related method before using GroovyScript");
-        }
-        URLClassLoader urlcl = null;
-        ClassLoader memoryProtectedLoader = null;
-        List<ClasspathEntry> cp = getClasspath();
-        if (!cp.isEmpty()) {
-            List<URL> urlList = new ArrayList<URL>(cp.size());
-            
-            for (ClasspathEntry entry : cp) {
-                ScriptApproval.get().using(entry);
-                urlList.add(entry.getURL());
-            }
-            
-            loader = urlcl = new URLClassLoader(urlList.toArray(new URL[urlList.size()]), loader);
+        try (PreparedScript scriptInstance = prepare(loader, binding)) {
+            return scriptInstance.run();
         }
-        boolean canDoCleanup = false;
+    }
 
-        try {
-            loader = GroovySandbox.createSecureClassLoader(loader);
+    /**
+     * Allows access to execute a script multiple times without preparation and clean-up overhead.
+     * While the intricate logic to do this continues to be hidden from users.
+     */
+    public final class PreparedScript implements Closeable {
+
+        private final GroovyShell sh;
+        private final Script preparedScript;
+        private ClassLoader memoryProtectedLoader = null;
+        private GroovyInterceptor scriptSandbox = null;
+        private URLClassLoader urlcl = null;
+        private boolean canDoCleanup = false;
+
+        /**
+         * @see @see SecureGroovyScript#evaluate()
+        */
+        @SuppressFBWarnings(value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "Managed by GroovyShell.")
+        private PreparedScript(ClassLoader loader, Binding binding) throws IllegalAccessException, IOException {
+            List<ClasspathEntry> cp = getClasspath();
+            if (!cp.isEmpty()) {
+                List<URL> urlList = new ArrayList<URL>(cp.size());
+
+                for (ClasspathEntry entry : cp) {
+                    ScriptApproval.get().using(entry);
+                    urlList.add(entry.getURL());
+                }
 
-            Field loaderF = null;
-            try {
-                loaderF = GroovyShell.class.getDeclaredField("loader");
-                loaderF.setAccessible(true);
-                canDoCleanup = true;
-            } catch (NoSuchFieldException nsme) {
-                LOGGER.log(Level.FINE, "GroovyShell fields have changed, field loader no longer exists -- memory leak fixes won't work");
+                loader = urlcl = new URLClassLoader(urlList.toArray(new URL[urlList.size()]), loader);
             }
 
-            GroovyShell sh;
-            if (sandbox) {
-                CompilerConfiguration cc = GroovySandbox.createSecureCompilerConfiguration();
-                sh = new GroovyShell(loader, binding, cc);
+            try {
+                loader = GroovySandbox.createSecureClassLoader(loader);
+                Field loaderF = null;
+                try {
+                    loaderF = GroovyShell.class.getDeclaredField("loader");
+                    loaderF.setAccessible(true);
+                    canDoCleanup = true;
+                } catch (NoSuchFieldException nsme) {
+                    LOGGER.log(Level.FINE, "GroovyShell fields have changed, field loader no longer exists -- memory leak fixes won't work");
+                }
 
-                if (canDoCleanup) {
-                    memoryProtectedLoader = new CleanGroovyClassLoader(loader, cc);
-                    loaderF.set(sh, memoryProtectedLoader);
+                if (sandbox) {
+                    CompilerConfiguration cc = GroovySandbox.createSecureCompilerConfiguration();
+                    sh = new GroovyShell(loader, binding, cc);
+
+                    if (canDoCleanup) {
+                        memoryProtectedLoader = new CleanGroovyClassLoader(loader, cc);
+                        loaderF.set(sh, memoryProtectedLoader);
+                    }
+
+                    preparedScript = sh.parse(script);
+                    scriptSandbox = GroovySandbox.createSandbox(preparedScript, Whitelist.all());
+                } else {
+                    sh = new GroovyShell(loader, binding);
+                    if (canDoCleanup) {
+                        memoryProtectedLoader = new CleanGroovyClassLoader(loader);
+                        loaderF.set(sh, memoryProtectedLoader);
+                    }
+
+                    preparedScript = sh.parse(ScriptApproval.get().using(script, GroovyLanguage.get()));
                 }
+            } catch (Exception e) {
+                close();
+                throw e;
+            }
+        }
 
+        public Object run() throws Exception {
+            if (sandbox) {
+                scriptSandbox.register();
                 try {
-                    return GroovySandbox.run(sh.parse(script), Whitelist.all());
+                    return preparedScript.run();
                 } catch (RejectedAccessException x) {
                     throw ScriptApproval.get().accessRejected(x, ApprovalContext.create());
+                } finally {
+                    scriptSandbox.unregister();
                 }
             } else {
-                sh = new GroovyShell(loader, binding);
-                if (canDoCleanup) {
-                    memoryProtectedLoader = new CleanGroovyClassLoader(loader);
-                    loaderF.set(sh, memoryProtectedLoader);
-                }
-                return sh.evaluate(ScriptApproval.get().using(script, GroovyLanguage.get()));
+                return preparedScript.run();
             }
+        }
 
-        } finally {
+        @Override
+        public void close() throws IOException {
             try {
                 if (canDoCleanup) {
                     cleanUpLoader(memoryProtectedLoader, new HashSet<ClassLoader>(), new HashSet<Class<?>>());

src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java

@@ -802,5 +802,32 @@ public void testSandboxClassResolution() throws Exception {
             assertTrue(e.getMessage().contains("staticMethod java.lang.System gc"));
         }
     }
-    
+
+    @Test public void testSandboxUsesSelectedBinding() throws Exception {
+        SecureGroovyScript sgs = new SecureGroovyScript("return a++", true, null);
+        Binding b = new Binding();
+        b.setVariable("a", 5);
+        try(SecureGroovyScript.PreparedScript script = sgs.configuringWithKeyItem().prepare(Jenkins.getInstance().getPluginManager().uberClassLoader, b)) {
+            Object res = script.run();
+            assertTrue((int) res == 5);
+            assertTrue((int) b.getVariable("a") == 6);
+            res = script.run();
+            assertTrue((int) res == 6);
+            assertTrue((int) b.getVariable("a") == 7);
+        }
+    }
+
+    @Test public void testNonSandboxUsesSelectedBinding() throws Exception {
+        SecureGroovyScript sgs = new SecureGroovyScript("return a++", false, null);
+        Binding b = new Binding();
+        b.setVariable("a", 5);
+        try (SecureGroovyScript.PreparedScript script = sgs.configuringWithKeyItem().prepare(Jenkins.getInstance().getPluginManager().uberClassLoader, b)) {
+            Object res = script.run();
+            assertTrue((int) res == 5);
+            assertTrue((int) b.getVariable("a") == 6);
+            res = script.run();
+            assertTrue((int) res == 6);
+            assertTrue((int) b.getVariable("a") == 7);
+        }
+    }
 }