Skip to content

Commit

Permalink
Merge pull request #68 from jglick/JENKINS-34739-varargs
Browse files Browse the repository at this point in the history 
[JENKINS-34739] Handle varargs
  • Loading branch information
@jglick
jglick committed May 19, 2016
2 parents 155ac3d + 02d9c3b commit 415e349
Show file tree
Hide file tree
Showing 3 changed files with 114 additions and 23 deletions.
55 changes: 34 additions & 21 deletions ...main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java
View file
Expand Up @@ -26,6 +26,7 @@

import com.google.common.primitives.Primitives;
import groovy.lang.GString;
import java.lang.reflect.Array;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
Expand All @@ -43,7 +44,10 @@
*/
class GroovyCallSiteSelector {

private static boolean matches(@Nonnull Class<?>[] parameterTypes, @Nonnull Object[] parameters) {
private static boolean matches(@Nonnull Class<?>[] parameterTypes, @Nonnull Object[] parameters, boolean varargs) {
if (varargs) {
parameters = parametersForVarargs(parameterTypes, parameters);
}
if (parameters.length != parameterTypes.length) {
return false;
}
Expand Down Expand Up @@ -80,6 +84,33 @@ && isInstancePrimitive(ClassUtils.primitiveToWrapper(parameterTypes[i]), paramet
return true;
}

/**
* Translates a method parameter list with varargs possibly spliced into the end into the actual parameters to be passed to the JVM call.
*/
private static Object[] parametersForVarargs(Class<?>[] parameterTypes, Object[] parameters) {
int fixedLen = parameterTypes.length - 1;
Class<?> componentType = parameterTypes[fixedLen].getComponentType();
assert componentType != null;
int arrayLength = parameters.length - fixedLen;
if (arrayLength >= 0) {
if (arrayLength == 1 && parameterTypes[fixedLen].isInstance(parameters[fixedLen])) {
// not a varargs call
return parameters;
} else {
Object array = Array.newInstance(componentType, arrayLength);
for (int i = 0; i < arrayLength; i++) {
Array.set(array, i, parameters[fixedLen + i]);
}
Object[] parameters2 = new Object[fixedLen + 1];
System.arraycopy(parameters, 0, parameters2, 0, fixedLen);
parameters2[fixedLen] = array;
return parameters2;
}
} else {
return parameters;
}
}

/**
* {@link Class#isInstance} extended to handle some important cases of primitive types.
*/
Expand Down Expand Up @@ -128,7 +159,7 @@ private static boolean isInstancePrimitive(@Nonnull Class<?> type, @Nonnull Obje

public static @CheckForNull Constructor<?> constructor(@Nonnull Class<?> receiver, @Nonnull Object[] args) {
for (Constructor<?> c : receiver.getDeclaredConstructors()) {
if (matches(c.getParameterTypes(), args)) {
if (matches(c.getParameterTypes(), args, c.isVarArgs())) {
return c;
}
}
Expand All @@ -140,28 +171,10 @@ private static boolean isInstancePrimitive(@Nonnull Class<?> type, @Nonnull Obje
return findMatchingMethod(receiver, method, args);
}

private static boolean isEnumInitializer(Method m) {
if (!m.getDeclaringClass().isEnum()) {
return false;
}
if (!m.isSynthetic()) {
return false;
}
final Class<?>[] parameterTypes = m.getParameterTypes();
if (parameterTypes.length != 1) {
return false;
}
final Class<?> type = parameterTypes[0];
if (!type.isArray()) {
return false;
}
return type.getComponentType().equals(Object.class);
}

private static Method findMatchingMethod(Class<?> receiver, String method, Object[] args) {
Method candidate = null;
for (Method m : receiver.getDeclaredMethods()) {
if (m.getName().equals(method) && (matches(m.getParameterTypes(), args) || isEnumInitializer(m))) {
if (m.getName().equals(method) && (matches(m.getParameterTypes(), args, m.isVarArgs()))) {
if (candidate == null || isMoreSpecific(m, candidate)) {
candidate = m;
}
Expand Down
6 changes: 6 additions & 0 deletions src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist
View file
Expand Up @@ -23,10 +23,13 @@ staticField groovy.lang.Closure OWNER_FIRST
staticField groovy.lang.Closure OWNER_ONLY
staticField groovy.lang.Closure TO_SELF
method groovy.lang.Closure call java.lang.Object
method groovy.lang.Closure call java.lang.Object[]
method groovy.lang.Closure curry java.lang.Object
method groovy.lang.Closure curry java.lang.Object[]
method groovy.lang.Closure getDelegate
method groovy.lang.Closure getResolveStrategy
method groovy.lang.Closure ncurry int java.lang.Object
method groovy.lang.Closure ncurry int java.lang.Object[]
method groovy.lang.Closure setDelegate java.lang.Object
method groovy.lang.Closure setResolveStrategy int
method groovy.lang.Script getBinding
Expand All @@ -40,6 +43,9 @@ new java.io.StringReader java.lang.String
method java.lang.CharSequence charAt int
method java.lang.CharSequence length
method java.lang.Comparable compareTo java.lang.Object
method java.lang.Enum name
method java.lang.Enum ordinal
# could add valueOf, though currently the staticField’s need to be whitelisted, which is the more likely use case
method java.lang.Object clone
method java.lang.Object equals java.lang.Object
method java.lang.Object getClass
Expand Down
76 changes: 74 additions & 2 deletions ...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java
View file
Expand Up @@ -69,7 +69,6 @@
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.ProxyWhitelist;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.StaticWhitelist;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
import org.junit.AssumptionViolatedException;
import org.junit.Ignore;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
Expand Down Expand Up @@ -431,7 +430,7 @@ private Unsafe() {}
new GroovyShell().evaluate("String.metaClass.getAnswer = {-> return 42}"); // privileged operation
assertEvaluate(new StaticWhitelist(), 42, "'existence'.getAnswer()");
assertEvaluate(new StaticWhitelist(), 42, "'existence'.answer");
assertRejected(new StaticWhitelist("method groovy.lang.Closure call java.lang.Object"), "staticMethod java.lang.System exit int", "def c = System.&exit; c(1)");
assertRejected(new GenericWhitelist(), "staticMethod java.lang.System exit int", "def c = System.&exit; c(1)");
}

@Issue("JENKINS-28277")
Expand All @@ -441,6 +440,59 @@ private Unsafe() {}
assertEvaluate(new GenericWhitelist(), 'h', "def charAt = {str, idx -> str.charAt(idx)}; def firstChar = charAt.ncurry(1, 0); firstChar 'hello'");
}

@Issue("JENKINS-34739")
@Test public void varargs() throws Exception {
// Control cases:
ProxyWhitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist());
assertEvaluate(wl, 0, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len(new String[0])");
assertEvaluate(wl, 3, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len(['one', 'two', 'three'] as String[])");
String uv = UsesVarargs.class.getName();
assertEvaluate(wl, 0, uv + ".len(new String[0])");
assertEvaluate(wl, 3, uv + ".len(['one', 'two', 'three'] as String[])");
assertEvaluate(wl, 0, uv + ".sum(new int[0])");
assertEvaluate(wl, 6, uv + ".sum([1, 2, 3] as int[])");
assertEvaluate(wl, 3, uv + ".xlen(3, new String[0])");
assertEvaluate(wl, 6, uv + ".xlen(3, ['one', 'two', 'three'] as String[])");
assertEvaluate(wl, "one,two,three", uv + ".join(',', ['one', 'two', 'three'] as String[])");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode(new String[0])");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode(['one', 'two', 'three'] as String[])");
// Test cases:
assertEvaluate(wl, 0, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len()");
assertEvaluate(wl, 3, "class UsesVarargs {static int len(String... vals) {vals.length}}; UsesVarargs.len('one', 'two', 'three')");
assertEvaluate(wl, 0, uv + ".len()");
assertEvaluate(wl, 3, uv + ".xlen(3)");
assertEvaluate(wl, 0, uv + ".sum()");
assertEvaluate(wl, 6, uv + ".sum(1, 2, 3)");
assertEvaluate(wl, 3, uv + ".len('one', 'two', 'three')");
assertEvaluate(wl, 6, uv + ".xlen(3, 'one', 'two', 'three')");
assertEvaluate(wl, "one,two,three", uv + ".join(',', 'one', 'two', 'three')");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode()");
assertRejected(wl, "staticMethod " + uv + " explode java.lang.String[]", uv + ".explode('one', 'two', 'three')");
}
public static class UsesVarargs {
@Whitelisted
public static int len(String... vals) {
return vals.length;
}
@Whitelisted
public static int sum(int... numbers) {
int sum = 0;
for (int number : numbers) {
sum += number;
}
return sum;
}
@Whitelisted
public static int xlen(int x, String... vals) {
return x + vals.length;
}
@Whitelisted
public static String join(String sep, String... vals) {
return StringUtils.join(vals, sep);
}
public static void explode(String... vals) {}
}

@Test public void templates() throws Exception {
final GroovyShell shell = new GroovyShell(GroovySandbox.createSecureCompilerConfiguration());
final Template t = new SimpleTemplateEngine(shell).createTemplate("goodbye <%= aspect.toLowerCase() %> world");
Expand Down Expand Up @@ -593,6 +645,26 @@ public Object invokeMethod(String name, Object args) {
+ "Thing.values()[0].description\n";
String expected = "The first thing";
assertEvaluate(new GenericWhitelist(), expected, script);
String e = E.class.getName();
ProxyWhitelist wl = new ProxyWhitelist(new GenericWhitelist(), new AnnotatedWhitelist());
assertEvaluate(wl, 2, e + ".TWO.getN()");
assertRejected(wl, "method " + e + " explode", e + ".TWO.explode()");
assertEvaluate(wl, "TWO", e + ".TWO.name()");
assertRejected(wl, "staticField " + e + " ONE", e + ".ONE.name()");
}
public enum E {
ONE(1),
@Whitelisted
TWO(2);
private final int n;
private E(int n) {
this.n = n;
}
@Whitelisted
public int getN() {
return n;
}
public void explode() {}
}

private static void assertEvaluate(Whitelist whitelist, final Object expected, final String script) {
Expand Down

0 comments on commit 415e349

Please sign in to comment.