Merge pull request #20 from jglick/GString-JENKINS-29541

[JENKINS-29541] GString receiver handling
jenkinsci · Aug 5, 2015 · ccca55d · ccca55d
2 parents 92e5ffe + 3142de1
commit ccca55d
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 7 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
@@ -89,14 +89,13 @@ private static boolean matches(@Nonnull Class<?>[] parameterTypes, @Nonnull Obje
      */
     public static @CheckForNull Method method(@Nonnull Object receiver, @Nonnull String method, @Nonnull Object[] args) {
         for (Class<?> c : types(receiver)) {
-            Method candidate = null;
-            for (Method m : c.getDeclaredMethods()) {
-                if (m.getName().equals(method) && matches(m.getParameterTypes(), args)) {
-                    if (candidate == null || isMoreSpecific(m, candidate)) {
-                        candidate = m;
-                    }
-                }
+            Method candidate = findMatchingMethod(c, method, args);
+            if (candidate != null) {
+                return candidate;
             }
+        }
+        if (receiver instanceof GString) { // cf. GString.invokeMethod
+            Method candidate = findMatchingMethod(String.class, method, args);
             if (candidate != null) {
                 return candidate;
             }
@@ -115,6 +114,10 @@ private static boolean matches(@Nonnull Class<?>[] parameterTypes, @Nonnull Obje
 
     public static @CheckForNull Method staticMethod(@Nonnull Class<?> receiver, @Nonnull String method, @Nonnull Object[] args) {
         // TODO should we check for inherited static calls?
+        return findMatchingMethod(receiver, method, args);
+    }
+
+    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)) {

diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/sandbox/whitelists/generic-whitelist
@@ -20,6 +20,7 @@ method java.lang.String trim
 method java.lang.String matches java.lang.String
 method java.lang.String split java.lang.String
 method java.lang.String startsWith java.lang.String
+method java.lang.String substring int int
 method java.lang.String replace java.lang.CharSequence java.lang.CharSequence
 method java.util.Collection contains java.lang.Object
 method java.util.concurrent.Callable call

diff --git a/.../java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java b/.../java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/GroovyCallSiteSelectorTest.java
@@ -25,11 +25,14 @@
 package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy;
 
 import com.google.common.io.NullOutputStream;
+import groovy.lang.GString;
 import java.io.PrintWriter;
 import java.lang.reflect.Method;
+import org.codehaus.groovy.runtime.GStringImpl;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.EnumeratingWhitelistTest;
 import static org.junit.Assert.*;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 
 public class GroovyCallSiteSelectorTest {
 
@@ -47,4 +50,12 @@ public class GroovyCallSiteSelectorTest {
         assertEquals(PrintWriter.class.getMethod("print", int.class), GroovyCallSiteSelector.method(receiver, "print", new Object[] {42}));
     }
 
+    @Issue("JENKINS-29541")
+    @Test public void methodsOnGString() throws Exception {
+        GStringImpl gString = new GStringImpl(new Object[0], new String[] {"x"});
+        assertEquals(String.class.getMethod("substring", int.class), GroovyCallSiteSelector.method(gString, "substring", new Object[] {99}));
+        assertEquals(GString.class.getMethod("getValues"), GroovyCallSiteSelector.method(gString, "getValues", new Object[0]));
+        assertEquals(GString.class.getMethod("getStrings"), GroovyCallSiteSelector.method(gString, "getStrings", new Object[0]));
+    }
+
 }
diff --git a/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java b/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java
@@ -88,6 +88,12 @@ public class SandboxInterceptorTest {
         assertEvaluate(new StaticWhitelist("new " + clazz, "method " + clazz + " specialize java.lang.Object", "method " + clazz + " quote java.lang.Object"), expected, script);
     }
 
+    @Issue("JENKINS-29541")
+    @Test public void substringGString() throws Exception {
+        assertEvaluate(new GenericWhitelist(), "hell", "'hello world'.substring(0, 4)");
+        assertEvaluate(new GenericWhitelist(), "hell", "def place = 'world'; \"hello ${place}\".substring(0, 4)");
+    }
+
     /**
      * Tests the proper interception of builder-like method.
      */