jenkinsci / groovy-sandbox

(Deprecated) Compile-time transformer to run Groovy code in a restrictive sandbox
MIT License
122 stars 60 forks source link

Need to handle Keyword Types #12

Closed shrtminded closed 10 years ago

shrtminded commented 10 years ago

I am receiving a IllegalArgumentException when I use a 'keyword' (like instanceof)

Example:

String myStr = "hello"; 
if (myStr instanceof String) {
  // do something
}

causes the following Exception

java.lang.IllegalArgumentException: 544
    at org.kohsuke.groovy.sandbox.impl.Ops.binaryOperatorMethods(Ops.java:29)
    at org.kohsuke.groovy.sandbox.impl.Checker.checkedBinaryOp(Checker.java:311)
    at org.kohsuke.groovy.sandbox.impl.Checker$checkedBinaryOp$1.callStatic(Unknown Source)
    at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallStatic(CallSiteArray.java:53)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:157)
    at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callStatic(AbstractCallSite.java:173)

It looks like KEYWORD types are not being checked.

I was able to work around this by adding a isKeywordOperator method in Ops.java

diff --git a/src/main/java/org/kohsuke/groovy/sandbox/impl/Ops.java b/src/main/java/org/kohsuke/groovy/sandbox/impl/Ops.java
index 1706438..f197d95 100644
--- a/src/main/java/org/kohsuke/groovy/sandbox/impl/Ops.java
+++ b/src/main/java/org/kohsuke/groovy/sandbox/impl/Ops.java
@@ -37,6 +37,10 @@ public class Ops {
     public static boolean isLogicalOperator(int type) {
         return Types.ofType(type,LOGICAL_OPERATOR);
     }
+    
+    public static boolean isKeywordOperator(int type) {
+        return Types.ofType(type, KEYWORD);
+    }

     // see http://groovy.codehaus.org/Operator+Overloading

and calling it in the SandboxTransformer

diff --git a/src/main/groovy/org/kohsuke/groovy/sandbox/SandboxTransformer.groovy b/src/main/groovy/org/kohsuke/groovy/sandbox/SandboxTransformer.groovy
index 5a13627..4a1c2bc 100644
--- a/src/main/groovy/org/kohsuke/groovy/sandbox/SandboxTransformer.groovy
+++ b/src/main/groovy/org/kohsuke/groovy/sandbox/SandboxTransformer.groovy
@@ -334,6 +334,8 @@ class SandboxTransformer extends CompilationCustomizer {
                                 transform(exp.rightExpression)
                         ])
                     }
+                } else if (Ops.isKeywordOperator(exp.operation.type)) {
+                   return super.transform(exp); 
                 } else
                 if (interceptMethodCall) {
                     // normally binary operators like a+b

Not sure if this is the best way to do this is or not but thought I would pass along what I found out.

michalbcz commented 10 years ago

isn't problem just in Ops because of missing bindings in binaryOperatorMethods ?

for example when using RegExp operator (which is missing in Ops declaration too) it ends with an error too:

def a = "blabla"
a =~ /bla/
kohsuke commented 10 years ago

instanceof and regexp operators added toward 1.6.