Skip to content

Commit

Permalink
Merge pull request #406 from offa/refactoring
Browse files Browse the repository at this point in the history 
Refactoring / modernization
  • Loading branch information
@jglick
jglick authored Apr 25, 2022
2 parents 7c1b73a + 753a4ab commit 99f78ba
Show file tree
Hide file tree
Showing 15 changed files with 110 additions and 229 deletions.
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/Whitelist.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,6 @@ public abstract class Whitelist implements ExtensionPoint {
}
return all;
}
private static final Map<Jenkins,Whitelist> allByJenkins = new WeakHashMap<Jenkins,Whitelist>();
private static final Map<Jenkins,Whitelist> allByJenkins = new WeakHashMap<>();

}
2 changes: 1 addition & 1 deletion ...main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ private static boolean isVarArgsMethod(@NonNull Method m, @NonNull Object[] args
}

private static Set<Class<?>> types(@NonNull Object o) {
Set<Class<?>> types = new LinkedHashSet<Class<?>>();
Set<Class<?>> types = new LinkedHashSet<>();
visitTypes(types, o.getClass());
return types;
}
Expand Down
86 changes: 14 additions & 72 deletions src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,7 @@ final class SandboxInterceptor extends GroovyInterceptor {
if (whitelist.permitsMethod(setterMethod, receiver, valueArg)) {
return super.onSetProperty(invoker, receiver, property, value);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectMethod(setterMethod);
}
};
rejector = () -> StaticWhitelist.rejectMethod(setterMethod);
}
}
Object[] propertyValueArgs = new Object[] {property, value};
Expand All @@ -216,23 +212,15 @@ final class SandboxInterceptor extends GroovyInterceptor {
if (whitelist.permitsMethod(setPropertyMethod, receiver, propertyValueArgs)) {
return super.onSetProperty(invoker, receiver, property, value);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectMethod(setPropertyMethod, receiver.getClass().getName() + "." + property);
}
};
rejector = () -> StaticWhitelist.rejectMethod(setPropertyMethod, receiver.getClass().getName() + "." + property);
}
}
final Field instanceField = GroovyCallSiteSelector.field(receiver, property);
if (instanceField != null) {
if (whitelist.permitsFieldSet(instanceField, receiver, value)) {
return super.onSetProperty(invoker, receiver, property, value);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectField(instanceField);
}
};
rejector = () -> StaticWhitelist.rejectField(instanceField);
}
}
if (receiver instanceof Class) {
Expand All @@ -241,23 +229,15 @@ final class SandboxInterceptor extends GroovyInterceptor {
if (whitelist.permitsStaticMethod(staticSetterMethod, valueArg)) {
return super.onSetProperty(invoker, receiver, property, value);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectStaticMethod(staticSetterMethod);
}
};
rejector = () -> StaticWhitelist.rejectStaticMethod(staticSetterMethod);
}
}
final Field staticField = GroovyCallSiteSelector.staticField((Class) receiver, property);
if (staticField != null) {
if (whitelist.permitsStaticFieldSet(staticField, value)) {
return super.onSetProperty(invoker, receiver, property, value);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectStaticField(staticField);
}
};
rejector = () -> StaticWhitelist.rejectStaticField(staticField);
}
}
}
Expand Down Expand Up @@ -285,11 +265,7 @@ final class SandboxInterceptor extends GroovyInterceptor {
if (whitelist.permitsMethod(getterMethod, receiver, noArgs)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectMethod(getterMethod);
}
};
rejector = () -> StaticWhitelist.rejectMethod(getterMethod);
}
}
String booleanGetter = "is" + Functions.capitalize(property);
Expand All @@ -298,11 +274,7 @@ final class SandboxInterceptor extends GroovyInterceptor {
if (whitelist.permitsMethod(booleanGetterMethod, receiver, noArgs)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectMethod(booleanGetterMethod);
}
};
rejector = () -> StaticWhitelist.rejectMethod(booleanGetterMethod);
}
}
// look for GDK methods
Expand All @@ -313,25 +285,15 @@ final class SandboxInterceptor extends GroovyInterceptor {
if (whitelist.permitsStaticMethod(dgmGetterMethod, selfArgs)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override
public RejectedAccessException reject() {
return StaticWhitelist.rejectStaticMethod(dgmGetterMethod);
}
};
rejector = () -> StaticWhitelist.rejectStaticMethod(dgmGetterMethod);
}
}
final Method dgmBooleanGetterMethod = GroovyCallSiteSelector.staticMethod(dgmClass, booleanGetter, selfArgs);
if (dgmBooleanGetterMethod != null && dgmBooleanGetterMethod.getReturnType() == boolean.class) {
if (whitelist.permitsStaticMethod(dgmBooleanGetterMethod, selfArgs)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override
public RejectedAccessException reject() {
return StaticWhitelist.rejectStaticMethod(dgmBooleanGetterMethod);
}
};
rejector = () -> StaticWhitelist.rejectStaticMethod(dgmBooleanGetterMethod);
}
}
}
Expand All @@ -340,11 +302,7 @@ public RejectedAccessException reject() {
if (whitelist.permitsFieldGet(instanceField, receiver)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectField(instanceField);
}
};
rejector = () -> StaticWhitelist.rejectField(instanceField);
}
}
// GroovyObject property access
Expand All @@ -354,11 +312,7 @@ public RejectedAccessException reject() {
if (whitelist.permitsMethod(getPropertyMethod, receiver, propertyArg)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectMethod(getPropertyMethod, receiver.getClass().getName() + "." + property);
}
};
rejector = () -> StaticWhitelist.rejectMethod(getPropertyMethod, receiver.getClass().getName() + "." + property);
}
}
MetaMethod metaMethod = findMetaMethod(receiver, getter, noArgs);
Expand All @@ -372,35 +326,23 @@ public RejectedAccessException reject() {
if (whitelist.permitsStaticMethod(staticGetterMethod, noArgs)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectStaticMethod(staticGetterMethod);
}
};
rejector = () -> StaticWhitelist.rejectStaticMethod(staticGetterMethod);
}
}
final Method staticBooleanGetterMethod = GroovyCallSiteSelector.staticMethod((Class) receiver, booleanGetter, noArgs);
if (staticBooleanGetterMethod != null && staticBooleanGetterMethod.getReturnType() == boolean.class) {
if (whitelist.permitsStaticMethod(staticBooleanGetterMethod, noArgs)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectStaticMethod(staticBooleanGetterMethod);
}
};
rejector = () -> StaticWhitelist.rejectStaticMethod(staticBooleanGetterMethod);
}
}
final Field staticField = GroovyCallSiteSelector.staticField((Class) receiver, property);
if (staticField != null) {
if (whitelist.permitsStaticFieldGet(staticField)) {
return super.onGetProperty(invoker, receiver, property);
} else if (rejector == null) {
rejector = new Rejector() {
@Override public RejectedAccessException reject() {
return StaticWhitelist.rejectStaticField(staticField);
}
};
rejector = () -> StaticWhitelist.rejectStaticField(staticField);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public Object evaluate(ClassLoader loader, Binding binding, @CheckForNull TaskLi
ClassLoader memoryProtectedLoader = null;
List<ClasspathEntry> cp = getClasspath();
if (!cp.isEmpty()) {
List<URL> urlList = new ArrayList<URL>(cp.size());
List<URL> urlList = new ArrayList<>(cp.size());

for (ClasspathEntry entry : cp) {
ScriptApproval.get().using(entry);
Expand Down Expand Up @@ -380,7 +380,7 @@ public Object evaluate(ClassLoader loader, Binding binding, @CheckForNull TaskLi
} finally {
try {
if (canDoCleanup) {
cleanUpLoader(memoryProtectedLoader, new HashSet<ClassLoader>(), new HashSet<Class<?>>());
cleanUpLoader(memoryProtectedLoader, new HashSet<>(), new HashSet<>());
}
} catch (Exception x) {
LOGGER.log(Level.WARNING, "failed to clean up memory " , x);
Expand Down
4 changes: 2 additions & 2 deletions ...in/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/EnumeratingWhitelist.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public abstract class EnumeratingWhitelist extends Whitelist {

protected abstract List<FieldSignature> staticFieldSignatures();

ConcurrentHashMap<String, Boolean> permittedCache = new ConcurrentHashMap<String, Boolean>(); // Not private to facilitate testing
ConcurrentHashMap<String, Boolean> permittedCache = new ConcurrentHashMap<>(); // Not private to facilitate testing

@SafeVarargs
private final void cacheSignatureList(List<Signature> ...sigs) {
Expand All @@ -83,7 +83,7 @@ final void precache() {
/** Frees up nearly all memory used for the cache. MUST BE CALLED if you change the result of the xxSignatures() methods. */
final void clearCache() {
this.permittedCache.clear();
this.permittedCache = new ConcurrentHashMap<String, Boolean>();
this.permittedCache = new ConcurrentHashMap<>();
}

@Override public final boolean permitsMethod(Method method, Object receiver, Object[] args) {
Expand Down
14 changes: 7 additions & 7 deletions src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/ProxyWhitelist.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,26 @@ public class ProxyWhitelist extends Whitelist {
private Collection<? extends Whitelist> originalDelegates;

@GuardedBy("lock")
final List<Whitelist> delegates = new ArrayList<Whitelist>();
final List<Whitelist> delegates = new ArrayList<>();

@GuardedBy("lock")
private final List<EnumeratingWhitelist.MethodSignature> methodSignatures = new ArrayList<EnumeratingWhitelist.MethodSignature>();
private final List<EnumeratingWhitelist.MethodSignature> methodSignatures = new ArrayList<>();

@GuardedBy("lock")
private final List<EnumeratingWhitelist.NewSignature> newSignatures = new ArrayList<EnumeratingWhitelist.NewSignature>();
private final List<EnumeratingWhitelist.NewSignature> newSignatures = new ArrayList<>();

@GuardedBy("lock")
private final List<EnumeratingWhitelist.MethodSignature> staticMethodSignatures = new ArrayList<EnumeratingWhitelist.MethodSignature>();
private final List<EnumeratingWhitelist.MethodSignature> staticMethodSignatures = new ArrayList<>();

@GuardedBy("lock")
private final List<EnumeratingWhitelist.FieldSignature> fieldSignatures = new ArrayList<EnumeratingWhitelist.FieldSignature>();
private final List<EnumeratingWhitelist.FieldSignature> fieldSignatures = new ArrayList<>();

@GuardedBy("lock")
private final List<EnumeratingWhitelist.FieldSignature> staticFieldSignatures = new ArrayList<EnumeratingWhitelist.FieldSignature>();
private final List<EnumeratingWhitelist.FieldSignature> staticFieldSignatures = new ArrayList<>();

/** anything wrapping us, so that we can propagate {@link #reset} calls up the chain */
@GuardedBy("lock")
private final Map<ProxyWhitelist,Void> wrappers = new WeakHashMap<ProxyWhitelist,Void>();
private final Map<ProxyWhitelist,Void> wrappers = new WeakHashMap<>();

// TODO Consider StampedLock when we switch to Java8 for better performance - https://dzone.com/articles/a-look-at-stampedlock
private ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
Expand Down
Loading

0 comments on commit 99f78ba

Please sign in to comment.