[JENKINS-37129] unclassified method vs MethodMissingException

As an user, I don't always precisely remember what methods an object has, an I often end up making mistakes. From workflow-cps perspective, this results in trying to call a method that does not exist. This must result in MethodMissingException, just like normal Groovy interpretor. The code currently makes this a RejectedAccessException, making the user believe that the method exists but the access it not allowed.
jenkinsci · Jul 14, 2017 · 950c4f4 · 950c4f4
1 parent 6b976f4
commit 950c4f4
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 5 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptor.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptor.java
@@ -26,6 +26,7 @@
 
 import groovy.lang.GroovyRuntimeException;
 import groovy.lang.MetaMethod;
+import groovy.lang.MissingMethodException;
 import groovy.lang.MissingPropertyException;
 import groovy.lang.Script;
 import hudson.Functions;
@@ -110,7 +111,8 @@ final class SandboxInterceptor extends GroovyInterceptor {
                 return super.onMethodCall(invoker, receiver, method, args);
             }
 
-            throw new RejectedAccessException("unclassified method " + EnumeratingWhitelist.getName(receiver.getClass()) + " " + method + printArgumentTypes(args));
+            // no such method exists
+            throw new MissingMethodException(method, receiver.getClass(), args);
         } else if (whitelist.permitsMethod(m, receiver, args)) {
             return super.onMethodCall(invoker, receiver, method, args);
         } else if (method.equals("invokeMethod") && args.length == 2 && args[0] instanceof String && args[1] instanceof Object[]) {

diff --git a/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java b/...test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SandboxInterceptorTest.java
@@ -24,10 +24,6 @@
 
 package org.jenkinsci.plugins.scriptsecurity.sandbox.groovy;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
 import groovy.json.JsonBuilder;
 import groovy.json.JsonDelegate;
 import groovy.lang.GString;
@@ -37,6 +33,7 @@
 import groovy.lang.GroovyShell;
 import groovy.lang.GroovySystem;
 import groovy.lang.MetaMethod;
+import groovy.lang.MissingMethodException;
 import groovy.lang.MissingPropertyException;
 import groovy.lang.Script;
 import groovy.text.SimpleTemplateEngine;
@@ -47,6 +44,7 @@
 import java.lang.reflect.Method;
 import java.net.URL;
 import java.text.DateFormat;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Date;
@@ -73,6 +71,9 @@
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
 
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
+
 public class SandboxInterceptorTest {
 
     @Test public void genericWhitelist() throws Exception {
@@ -698,4 +699,29 @@ private static void assertRejected(Whitelist whitelist, String expectedSignature
         }
     }
 
+    @Issue("JENKINS-37129")
+    @Test public void methodMissingException() throws Exception {
+        // the case where the unsafe receiver type causes the security check to fail
+        try {
+            assertEvaluate(new GenericWhitelist(), "should fail", "[].noSuchMethod()");
+        } catch (MissingMethodException e) {
+            assertEquals(e.getType(),ArrayList.class);
+            assertThat(e.getMethod(),is("noSuchMethod"));
+        }
+
+        // trying to call an existing method that's not safe
+        try {
+            assertEvaluate(new GenericWhitelist(), "should fail", "[].class.classLoader");
+        } catch (RejectedAccessException e) {
+            assertEquals("method java.lang.Class getClassLoader", e.getSignature());
+        }
+
+        // the case where the safe receiver type causes the security check to pass
+        try {
+            assertEvaluate(new GenericWhitelist(), "should fail", "1.noSuchMethod()");
+        } catch (MissingMethodException e) {
+            assertEquals(e.getType(),Integer.class);
+            assertThat(e.getMethod(),is("noSuchMethod"));
+        }
+    }
 }