Skip to content

Commit 56ae4c8

Browse files
committed
script performant multi-run support
1 parent 7ff8aed commit 56ae4c8

File tree

3 files changed

+239
-150
lines changed

3 files changed

+239
-150
lines changed

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public class GroovySandbox {
7272
public static @Nonnull ClassLoader createSecureClassLoader(ClassLoader base) {
7373
return new SandboxResolvingClassLoader(base);
7474
}
75-
75+
7676
/**
7777
* Runs a block in the sandbox.
7878
* You must have used {@link #createSecureCompilerConfiguration} to prepare the Groovy shell.
@@ -132,10 +132,7 @@ public void run() {
132132
* @throws RejectedAccessException in case an attempted call was not whitelisted
133133
*/
134134
public static Object run(@Nonnull Script script, @Nonnull final Whitelist whitelist) throws RejectedAccessException {
135-
Whitelist wrapperWhitelist = new ProxyWhitelist(
136-
new ClassLoaderWhitelist(script.getClass().getClassLoader()),
137-
whitelist);
138-
GroovyInterceptor sandbox = new SandboxInterceptor(wrapperWhitelist);
135+
GroovyInterceptor sandbox = createSandbox(script, whitelist);
139136
sandbox.register();
140137
try {
141138
return script.run();
@@ -144,6 +141,20 @@ public static Object run(@Nonnull Script script, @Nonnull final Whitelist whitel
144141
}
145142
}
146143

144+
/**
145+
* Prepares a sandbox for a script.
146+
* You must have used {@link #createSecureCompilerConfiguration} to prepare the Groovy shell.
147+
* @param script a script ready to {@link Script#run}, created for example by {@link GroovyShell#parse(String)}
148+
* @param whitelist the whitelist to use, such as {@link Whitelist#all()}
149+
* @return the sandbox for running this script
150+
*/
151+
public static GroovyInterceptor createSandbox(@Nonnull Script script, @Nonnull final Whitelist whitelist) {
152+
Whitelist wrapperWhitelist = new ProxyWhitelist(
153+
new ClassLoaderWhitelist(script.getClass().getClassLoader()),
154+
whitelist);
155+
return new SandboxInterceptor(wrapperWhitelist);
156+
}
157+
147158
private GroovySandbox() {}
148159

149160
}

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

Lines changed: 85 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import groovy.lang.Binding;
2929
import groovy.lang.GroovyClassLoader;
3030
import groovy.lang.GroovyShell;
31+
import groovy.lang.Script;
3132
import hudson.Extension;
3233
import hudson.PluginManager;
3334
import hudson.model.AbstractDescribableImpl;
@@ -36,6 +37,7 @@
3637
import hudson.util.FormValidation;
3738

3839
import java.beans.Introspector;
40+
import java.io.IOException;
3941
import java.lang.ref.Reference;
4042
import java.lang.ref.WeakReference;
4143
import java.lang.reflect.Field;
@@ -68,6 +70,7 @@
6870
import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedClasspathException;
6971
import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException;
7072
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
73+
import org.kohsuke.groovy.sandbox.GroovyInterceptor;
7174
import org.kohsuke.stapler.DataBoundConstructor;
7275
import org.kohsuke.stapler.QueryParameter;
7376
import org.kohsuke.stapler.Stapler;
@@ -80,7 +83,7 @@
8083
* Use {@code <f:property field="…"/>} to configure it from Jelly.
8184
*/
8285
public final class SecureGroovyScript extends AbstractDescribableImpl<SecureGroovyScript> {
83-
86+
8487
private final @Nonnull String script;
8588
private final boolean sandbox;
8689
private final @CheckForNull List<ClasspathEntry> classpath;
@@ -277,6 +280,10 @@ private static void cleanUpObjectStreamClassCaches(@Nonnull Class<?> clazz) thro
277280
}
278281
}
279282

283+
public PreparedScript prepare(ClassLoader loader, Binding binding) throws IllegalAccessException, IOException {
284+
return new PreparedScript(loader, binding);
285+
}
286+
280287
/**
281288
* Runs the Groovy script, using the sandbox if so configured.
282289
* @param loader a class loader for constructing the shell, such as {@link PluginManager#uberClassLoader} (will be augmented by {@link #getClasspath} if nonempty)
@@ -287,63 +294,99 @@ private static void cleanUpObjectStreamClassCaches(@Nonnull Class<?> clazz) thro
287294
* @throws UnapprovedUsageException in case of a non-sandbox issue
288295
* @throws UnapprovedClasspathException in case some unapproved classpath entries were requested
289296
*/
290-
@SuppressFBWarnings(value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "Managed by GroovyShell.")
291297
public Object evaluate(ClassLoader loader, Binding binding) throws Exception {
292-
if (!calledConfiguring) {
293-
throw new IllegalStateException("you need to call configuring or a related method before using GroovyScript");
294-
}
295-
URLClassLoader urlcl = null;
296-
ClassLoader memoryProtectedLoader = null;
297-
List<ClasspathEntry> cp = getClasspath();
298-
if (!cp.isEmpty()) {
299-
List<URL> urlList = new ArrayList<URL>(cp.size());
300-
301-
for (ClasspathEntry entry : cp) {
302-
ScriptApproval.get().using(entry);
303-
urlList.add(entry.getURL());
304-
}
305-
306-
loader = urlcl = new URLClassLoader(urlList.toArray(new URL[urlList.size()]), loader);
307-
}
308-
boolean canDoCleanup = false;
298+
PreparedScript scriptInstance = prepare(loader, binding);
309299

310300
try {
311-
loader = GroovySandbox.createSecureClassLoader(loader);
301+
return scriptInstance.run();
302+
} finally {
303+
scriptInstance.cleanUp();
304+
}
305+
}
312306

313-
Field loaderF = null;
314-
try {
315-
loaderF = GroovyShell.class.getDeclaredField("loader");
316-
loaderF.setAccessible(true);
317-
canDoCleanup = true;
318-
} catch (NoSuchFieldException nsme) {
319-
LOGGER.log(Level.FINE, "GroovyShell fields have changed, field loader no longer exists -- memory leak fixes won't work");
307+
/**
308+
* Allows access to execue a script multiple times without preparation and clean-up overhead.
309+
* While the intricate logic to do this continues to be hidden from users.
310+
*/
311+
public final class PreparedScript {
312+
313+
private final GroovyShell sh;
314+
private final Script preparedScript;
315+
private ClassLoader memoryProtectedLoader = null;
316+
private GroovyInterceptor scriptSandbox = null;
317+
private URLClassLoader urlcl = null;
318+
private boolean canDoCleanup = false;
319+
320+
/**
321+
* @see @see SecureGroovyScript#evaluate()
322+
*/
323+
@SuppressFBWarnings(value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED", justification = "Managed by GroovyShell.")
324+
private PreparedScript(ClassLoader loader, Binding binding) throws IllegalAccessException, IOException {
325+
List<ClasspathEntry> cp = getClasspath();
326+
if (!cp.isEmpty()) {
327+
List<URL> urlList = new ArrayList<URL>(cp.size());
328+
329+
for (ClasspathEntry entry : cp) {
330+
ScriptApproval.get().using(entry);
331+
urlList.add(entry.getURL());
332+
}
333+
334+
loader = urlcl = new URLClassLoader(urlList.toArray(new URL[urlList.size()]), loader);
320335
}
321336

322-
GroovyShell sh;
323-
if (sandbox) {
324-
CompilerConfiguration cc = GroovySandbox.createSecureCompilerConfiguration();
325-
sh = new GroovyShell(loader, binding, cc);
337+
try {
338+
loader = GroovySandbox.createSecureClassLoader(loader);
339+
Field loaderF = null;
340+
try {
341+
loaderF = GroovyShell.class.getDeclaredField("loader");
342+
loaderF.setAccessible(true);
343+
canDoCleanup = true;
344+
} catch (NoSuchFieldException nsme) {
345+
LOGGER.log(Level.FINE, "GroovyShell fields have changed, field loader no longer exists -- memory leak fixes won't work");
346+
}
326347

327-
if (canDoCleanup) {
328-
memoryProtectedLoader = new CleanGroovyClassLoader(loader, cc);
329-
loaderF.set(sh, memoryProtectedLoader);
348+
if (sandbox) {
349+
CompilerConfiguration cc = GroovySandbox.createSecureCompilerConfiguration();
350+
sh = new GroovyShell(loader, binding, cc);
351+
352+
if (canDoCleanup) {
353+
memoryProtectedLoader = new CleanGroovyClassLoader(loader, cc);
354+
loaderF.set(sh, memoryProtectedLoader);
355+
}
356+
357+
preparedScript = sh.parse(script);
358+
scriptSandbox = GroovySandbox.createSandbox(preparedScript, Whitelist.all());
359+
} else {
360+
sh = new GroovyShell(loader, binding);
361+
if (canDoCleanup) {
362+
memoryProtectedLoader = new CleanGroovyClassLoader(loader);
363+
loaderF.set(sh, memoryProtectedLoader);
364+
}
365+
366+
preparedScript = sh.parse(ScriptApproval.get().using(script, GroovyLanguage.get()));
330367
}
368+
} catch (Exception e) {
369+
cleanUp();
370+
throw e;
371+
}
372+
}
331373

374+
public Object run() throws Exception {
375+
if (sandbox) {
376+
scriptSandbox.register();
332377
try {
333-
return GroovySandbox.run(sh.parse(script), Whitelist.all());
378+
return preparedScript.run();
334379
} catch (RejectedAccessException x) {
335380
throw ScriptApproval.get().accessRejected(x, ApprovalContext.create());
381+
} finally {
382+
scriptSandbox.unregister();
336383
}
337384
} else {
338-
sh = new GroovyShell(loader, binding);
339-
if (canDoCleanup) {
340-
memoryProtectedLoader = new CleanGroovyClassLoader(loader);
341-
loaderF.set(sh, memoryProtectedLoader);
342-
}
343-
return sh.evaluate(ScriptApproval.get().using(script, GroovyLanguage.get()));
385+
return preparedScript.run();
344386
}
387+
}
345388

346-
} finally {
389+
public void cleanUp() throws IOException {
347390
try {
348391
if (canDoCleanup) {
349392
cleanUpLoader(memoryProtectedLoader, new HashSet<ClassLoader>(), new HashSet<Class<?>>());

0 commit comments

Comments
 (0)