From 08c3aeab9b283181c84baf487f21184d2bc97f86 Mon Sep 17 00:00:00 2001 From: Jeoffrey Haeyaert Date: Fri, 17 Dec 2021 16:27:21 +0100 Subject: [PATCH] feat(perf): adapt policy for new classloader system Closes gravitee-io/issues#6758 --- pom.xml | 8 +++ .../policy/groovy/GroovyInitializer.java | 10 +++- .../groovy/sandbox/SecuredGroovyShell.java | 49 ++++++++++++------- .../groovy/sandbox/SecuredResolver.java | 18 ++++--- .../io/gravitee/policy/groovy/utils/Sha1.java | 5 +- 5 files changed, 60 insertions(+), 30 deletions(-) diff --git a/pom.xml b/pom.xml index 964a408..693116f 100644 --- a/pom.xml +++ b/pom.xml @@ -42,6 +42,7 @@ 3.0.9 1.27 3.11 + 30.1.1-jre 2.5.5 @@ -123,6 +124,13 @@ provided + + com.google.guava + guava + ${guava.version} + provided + + junit diff --git a/src/main/java/io/gravitee/policy/groovy/GroovyInitializer.java b/src/main/java/io/gravitee/policy/groovy/GroovyInitializer.java index 275556c..a948079 100644 --- a/src/main/java/io/gravitee/policy/groovy/GroovyInitializer.java +++ b/src/main/java/io/gravitee/policy/groovy/GroovyInitializer.java @@ -28,19 +28,25 @@ public class GroovyInitializer implements PolicyContext, PolicyContextProviderAware { private Environment environment; + private boolean classLoaderLegacyMode = true; @Override public void onActivation() throws Exception { - SecuredResolver.initialize(this.environment); + if (classLoaderLegacyMode || !SecuredResolver.isInitialized()) { + SecuredResolver.initialize(this.environment); + } } @Override public void onDeactivation() throws Exception { - SecuredResolver.destroy(); + if (classLoaderLegacyMode) { + SecuredResolver.destroy(); + } } @Override public void setPolicyContextProvider(PolicyContextProvider policyContextProvider) { this.environment = policyContextProvider.getComponent(Environment.class); + this.classLoaderLegacyMode = environment.getProperty("classloader.legacy.enabled", Boolean.class, true); } } diff --git a/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredGroovyShell.java b/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredGroovyShell.java index 19ff1d0..022eb6c 100644 --- a/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredGroovyShell.java +++ b/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredGroovyShell.java @@ -15,25 +15,18 @@ */ package io.gravitee.policy.groovy.sandbox; -import static org.codehaus.groovy.ast.tools.GeneralUtils.classX; -import static org.codehaus.groovy.ast.tools.GeneralUtils.propX; - +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import groovy.lang.Binding; import groovy.lang.GroovyCodeSource; import groovy.lang.GroovyShell; import groovy.lang.Script; -import groovy.transform.ThreadInterrupt; -import groovy.transform.TimedInterrupt; import io.gravitee.policy.groovy.GroovyPolicy; import io.gravitee.policy.groovy.utils.Sha1; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.TimeUnit; +import java.time.Duration; import org.apache.groovy.json.internal.FastStringUtils; -import org.apache.groovy.util.Maps; import org.codehaus.groovy.control.CompilationFailedException; import org.codehaus.groovy.control.CompilerConfiguration; -import org.codehaus.groovy.control.customizers.ASTTransformationCustomizer; import org.codehaus.groovy.control.customizers.SecureASTCustomizer; import org.codehaus.groovy.runtime.InvokerHelper; import org.kohsuke.groovy.sandbox.GroovyInterceptor; @@ -44,6 +37,12 @@ * @author GraviteeSource Team */ public class SecuredGroovyShell { + + /** + * Number of hours to keep compiled script in cache after the last time it was accessed. + */ + private static final int CODE_CACHE_EXPIRATION_HOURS = 1; + static { // Do not change this block of code which is required to work with groovy 2.5 and the classloader used // to load services @@ -53,11 +52,13 @@ public class SecuredGroovyShell { Thread.currentThread().setContextClassLoader(loader); } - private GroovyShell groovyShell; - private ConcurrentMap> sources = new ConcurrentHashMap<>(); - private GroovyInterceptor groovyInterceptor; + private final GroovyShell groovyShell; + private final Cache> sources; + private final GroovyInterceptor groovyInterceptor; public SecuredGroovyShell() { + this.sources = CacheBuilder.newBuilder().expireAfterAccess(Duration.ofHours(CODE_CACHE_EXPIRATION_HOURS)).build(); + CompilerConfiguration conf = new CompilerConfiguration(); // Add Kohsuke's sandbox transformer which will delegate calls to SecuredInterceptor. @@ -101,12 +102,22 @@ public T evaluate(String script, Binding binding) { private Class getOrCreate(String script) throws CompilationFailedException { String key = Sha1.sha1(script); - return sources.computeIfAbsent( - key, - s -> { - GroovyCodeSource gcs = new GroovyCodeSource(script, key, GroovyShell.DEFAULT_CODE_BASE); - return groovyShell.getClassLoader().parseClass(gcs, true); + try { + return sources.get( + key, + () -> { + GroovyCodeSource gcs = new GroovyCodeSource(script, key, GroovyShell.DEFAULT_CODE_BASE); + return groovyShell.getClassLoader().parseClass(gcs, true); + } + ); + } catch (Exception e) { + final Throwable cause = e.getCause(); + if (cause instanceof CompilationFailedException) { + throw (CompilationFailedException) cause; + } else if (cause instanceof SecurityException) { + throw (SecurityException) cause; } - ); + throw new RuntimeException("Unable to compile script", e); + } } } diff --git a/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredResolver.java b/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredResolver.java index c8c7d06..383c8cb 100644 --- a/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredResolver.java +++ b/src/main/java/io/gravitee/policy/groovy/sandbox/SecuredResolver.java @@ -107,13 +107,19 @@ public class SecuredResolver { private final Map, List> methodsByTypeAndSuperTypes; private final Map resolved; - public static void initialize(@Nullable Environment environment) { - loadWhitelist(environment); - INSTANCE = new SecuredResolver(); + public static synchronized void initialize(@Nullable Environment environment) { + if (!isInitialized()) { + loadWhitelist(environment); + INSTANCE = new SecuredResolver(); + } + } + + public static synchronized boolean isInitialized() { + return INSTANCE != null; } - public static void destroy() { - if (INSTANCE != null) { + public static synchronized void destroy() { + if (isInitialized()) { methodsByType.clear(); fieldsByType.clear(); constructorsByType.clear(); @@ -123,7 +129,7 @@ public static void destroy() { } public static SecuredResolver getInstance() { - if (INSTANCE == null) { + if (!isInitialized()) { initialize(null); } diff --git a/src/main/java/io/gravitee/policy/groovy/utils/Sha1.java b/src/main/java/io/gravitee/policy/groovy/utils/Sha1.java index cb69ab7..cc0fee2 100644 --- a/src/main/java/io/gravitee/policy/groovy/utils/Sha1.java +++ b/src/main/java/io/gravitee/policy/groovy/utils/Sha1.java @@ -41,13 +41,12 @@ private static String convertToHex(byte[] data) { public static String sha1(String text) { try { MessageDigest md = MessageDigest.getInstance("SHA-1"); - byte[] sha1hash = new byte[40]; + byte[] sha1hash; md.update(text.getBytes("UTF-8"), 0, text.length()); sha1hash = md.digest(); return convertToHex(sha1hash); } catch (Exception ex) { - ex.printStackTrace(); - return null; + throw new RuntimeException(ex); } } }