[FIXED JENKINS-46213] Treat trailing array params as varargs

Groovy's method lookup logic will treat `sprintf(fmt, "string", "other string")` the same as `sprintf(fmt, ["string", "other string"])`. We should emulate that.
jenkinsci · Dec 6, 2017 · 66a5ea8 · 66a5ea8
1 parent ab44ba3
commit 66a5ea8
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 2 deletions.
diff --git a/...main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java b/...main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelector.java
@@ -195,16 +195,50 @@ private static boolean isInstancePrimitive(@Nonnull Class<?> type, @Nonnull Obje
 
     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, m.isVarArgs()))) {
-                if (candidate == null || isMoreSpecific(m, m.getParameterTypes(), m.isVarArgs(), candidate, candidate.getParameterTypes(), candidate.isVarArgs())) {
+            if (m.getName().equals(method) && (matches(m.getParameterTypes(), args, isVargsMethod(m, args)))) {
+                if (candidate == null || isMoreSpecific(m, m.getParameterTypes(), isVargsMethod(m, args), candidate,
+                        candidate.getParameterTypes(), isVargsMethod(candidate, args))) {
                     candidate = m;
                 }
             }
         }
         return candidate;
     }
 
+    /**
+     * Emulates, with some tweaks, {@link org.codehaus.groovy.reflection.ParameterTypes#isVargsMethod(Object[])}
+     */
+    private static boolean isVargsMethod(Method m, Object[] args) {
+        if (m.isVarArgs()) {
+            return true;
+        }
+        Class<?>[] paramTypes = m.getParameterTypes();
+
+        // If there's 0 or only 1 parameter type, we don't want to do varargs magic. Normal callsite selector logic works then.
+        if (paramTypes.length < 2) {
+            return false;
+        }
+
+        int lastIndex = paramTypes.length - 1;
+        // If there are more arguments than parameter types and the last parameter type is an array, we may be vargy.
+        if (paramTypes[lastIndex].isArray() && args.length > paramTypes.length) {
+            Class<?> lastClass = paramTypes[lastIndex].getComponentType();
+            // Check each possible vararg to see if it can be cast to the array's component type. If not, we're not vargy.
+            for (int i = lastIndex; i < args.length; i++) {
+                if (!lastClass.isAssignableFrom(args[i].getClass())) {
+                    return false;
+                }
+            }
+            // Otherwise, we are.
+            return true;
+        }
+
+        // Fallback to we're not vargy.
+        return false;
+    }
+
     public static @CheckForNull Field field(@Nonnull Object receiver, @Nonnull String field) {
         for (Class<?> c : types(receiver)) {
             for (Field f : c.getDeclaredFields()) {

diff --git a/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java b/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java
@@ -969,4 +969,28 @@ public void varArgsWithOtherArgs() throws Exception {
         String expected = "4-true-a-b-c";
         assertEvaluate(wl, expected, script);
     }
+
+    @Issue("JENKINS-46213")
+    @Test
+    public void varArgsOnStaticDeclaration() throws Exception {
+        String script = "class Explode {\n" +
+                "  static TEST_FMT = 'a:%s b:%s'\n" +
+                "  static STATIC_TEST = sprintf(\n" +
+                "    TEST_FMT,\n" +
+                "    '1',\n" +
+                "    '2',\n" +
+                "  )\n" +
+                "  String fieldTest = sprintf(\n" +
+                "    TEST_FMT,\n" +
+                "    '3',\n" +
+                "    '4',\n" +
+                "  )\n" +
+                "}\n" +
+                "def ex = new Explode()\n" +
+                "return \"${Explode.STATIC_TEST} ${ex.fieldTest}\"\n";
+
+        assertEvaluate(new StaticWhitelist("staticMethod org.codehaus.groovy.runtime.DefaultGroovyMethods sprintf java.lang.Object java.lang.String java.lang.Object[]"),
+                "a:1 b:2 a:3 b:4",
+                script);
+    }
 }