You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2019/11/24 23:25:54 UTC

[groovy] branch master updated: GROOVY-9318: add support for ** syntax in star import white/black lists

This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new 4483012  GROOVY-9318: add support for ** syntax in star import white/black lists
4483012 is described below

commit 4483012e08ed4115bf25280260d7b2e3c95efeed
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sun Nov 24 17:25:34 2019 -0600

    GROOVY-9318: add support for ** syntax in star import white/black lists
    
    This closes #1100
    This closes #1101
---
 .../control/customizers/SecureASTCustomizer.java   |  84 ++--
 .../customizers/SecureASTCustomizerTest.groovy     | 449 +++++++++++++--------
 2 files changed, 315 insertions(+), 218 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java b/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
index 5f2c2c4..572d596 100644
--- a/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
+++ b/src/main/java/org/codehaus/groovy/control/customizers/SecureASTCustomizer.java
@@ -18,7 +18,6 @@
  */
 package org.codehaus.groovy.control.customizers;
 
-import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.CodeVisitorSupport;
 import org.codehaus.groovy.ast.GroovyCodeVisitor;
@@ -176,14 +175,14 @@ import java.util.Map;
  *             config.addCompilationCustomizers(imports, secure)
  *             GroovyClassLoader loader = new GroovyClassLoader(this.class.classLoader, config)
  *  </pre>
- *  
+ *
  * @since 1.8.0
  */
 public class SecureASTCustomizer extends CompilationCustomizer {
 
     private boolean isPackageAllowed = true;
-    private boolean isMethodDefinitionAllowed = true;
     private boolean isClosuresAllowed = true;
+    private boolean isMethodDefinitionAllowed = true;
 
     // imports
     private List<String> importsWhitelist;
@@ -201,7 +200,6 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     private List<String> staticStarImportsWhitelist;
     private List<String> staticStarImportsBlacklist;
 
-
     // indirect import checks
     // if set to true, then security rules on imports will also be applied on classnodes.
     // Direct instantiation of classes without imports will therefore also fail if this option is enabled
@@ -210,12 +208,12 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     // statements
     private List<Class<? extends Statement>> statementsWhitelist;
     private List<Class<? extends Statement>> statementsBlacklist;
-    private final List<StatementChecker> statementCheckers = new LinkedList<StatementChecker>();
+    private final List<StatementChecker> statementCheckers = new LinkedList<>();
 
     // expressions
     private List<Class<? extends Expression>> expressionsWhitelist;
     private List<Class<? extends Expression>> expressionsBlacklist;
-    private final List<ExpressionChecker> expressionCheckers = new LinkedList<ExpressionChecker>();
+    private final List<ExpressionChecker> expressionCheckers = new LinkedList<>();
 
     // tokens from Types
     private List<Integer> tokensWhitelist;
@@ -303,14 +301,13 @@ public class SecureASTCustomizer extends CompilationCustomizer {
         if (this.importsWhitelist == null) importsWhitelist = Collections.emptyList();
     }
 
-    /**
-     * Ensures that every star import ends with .* as this is the expected syntax in import checks.
-     */
     private static List<String> normalizeStarImports(List<String> starImports) {
-        List<String> result = new ArrayList<String>(starImports.size());
+        List<String> result = new ArrayList<>(starImports.size());
         for (String starImport : starImports) {
             if (starImport.endsWith(".*")) {
                 result.add(starImport);
+            } else if (starImport.endsWith("**")) {
+                result.add(starImport.replaceFirst("\\*+$", ""));
             } else if (starImport.endsWith(".")) {
                 result.add(starImport + "*");
             } else {
@@ -493,7 +490,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param constantTypesWhiteList a list of classes.
      */
     public void setConstantTypesClassesWhiteList(final List<Class> constantTypesWhiteList) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : constantTypesWhiteList) {
             values.add(aClass.getName());
         }
@@ -506,7 +503,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param constantTypesBlackList a list of classes.
      */
     public void setConstantTypesClassesBlackList(final List<Class> constantTypesBlackList) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : constantTypesBlackList) {
             values.add(aClass.getName());
         }
@@ -519,8 +516,8 @@ public class SecureASTCustomizer extends CompilationCustomizer {
 
     /**
      * Sets the list of classes which deny method calls.
-     * 
-     * Please note that since Groovy is a dynamic language, and 
+     *
+     * Please note that since Groovy is a dynamic language, and
      * this class performs a static type check, it will be reletively
      * simple to bypass any blacklist unless the receivers blacklist contains, at
      * a minimum, Object, Script, GroovyShell, and Eval. Additionally,
@@ -543,7 +540,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param receiversBlacklist a list of classes.
      */
     public void setReceiversClassesBlackList(final List<Class> receiversBlacklist) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : receiversBlacklist) {
             values.add(aClass.getName());
         }
@@ -572,7 +569,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
      * @param receiversWhitelist a list of classes.
      */
     public void setReceiversClassesWhiteList(final List<Class> receiversWhitelist) {
-        List<String> values = new LinkedList<String>();
+        List<String> values = new LinkedList<>();
         for (Class aClass : receiversWhitelist) {
             values.add(aClass.getName());
         }
@@ -581,7 +578,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
 
     @Override
     public void call(final SourceUnit source, final GeneratorContext context, final ClassNode classNode) throws CompilationFailedException {
-        final ModuleNode ast = source.getAST();
+        ModuleNode ast = source.getAST();
         if (!isPackageAllowed && ast.getPackage() != null) {
             throw new SecurityException("Package definitions are not allowed");
         }
@@ -590,12 +587,10 @@ public class SecureASTCustomizer extends CompilationCustomizer {
         // verify imports
         if (importsBlacklist != null || importsWhitelist != null || starImportsBlacklist != null || starImportsWhitelist != null) {
             for (ImportNode importNode : ast.getImports()) {
-                final String className = importNode.getClassName();
-                assertImportIsAllowed(className);
+                assertImportIsAllowed(importNode.getClassName());
             }
             for (ImportNode importNode : ast.getStarImports()) {
-                final String className = importNode.getPackageName();
-                assertStarImportIsAllowed(className + "*");
+                assertStarImportIsAllowed(importNode.getPackageName() + "*");
             }
         }
 
@@ -611,7 +606,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
             }
         }
 
-        final GroovyCodeVisitor visitor = createGroovyCodeVisitor();
+        GroovyCodeVisitor visitor = createGroovyCodeVisitor();
         ast.getStatementBlock().visit(visitor);
         for (ClassNode clNode : ast.getClasses()) {
             if (clNode!=classNode) {
@@ -627,7 +622,9 @@ public class SecureASTCustomizer extends CompilationCustomizer {
         List<MethodNode> methods = filterMethods(classNode);
         if (isMethodDefinitionAllowed) {
             for (MethodNode method : methods) {
-                if (method.getDeclaringClass()==classNode && method.getCode() != null) method.getCode().visit(visitor);
+                if (method.getDeclaringClass() == classNode && method.getCode() != null) {
+                    method.getCode().visit(visitor);
+                }
             }
         }
     }
@@ -643,7 +640,7 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     }
 
     protected static List<MethodNode> filterMethods(ClassNode owner) {
-        List<MethodNode> result = new LinkedList<MethodNode>();
+        List<MethodNode> result = new LinkedList<>();
         List<MethodNode> methods = owner.getMethods();
         for (MethodNode method : methods) {
             if (method.getDeclaringClass() == owner && !method.isSynthetic()) {
@@ -655,37 +652,40 @@ public class SecureASTCustomizer extends CompilationCustomizer {
     }
 
     protected void assertStarImportIsAllowed(final String packageName) {
-        if (starImportsWhitelist != null && !starImportsWhitelist.contains(packageName)) {
+        if (starImportsWhitelist != null && !(starImportsWhitelist.contains(packageName)
+                || starImportsWhitelist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith))) {
             throw new SecurityException("Importing [" + packageName + "] is not allowed");
         }
-        if (starImportsBlacklist != null && starImportsBlacklist.contains(packageName)) {
+        if (starImportsBlacklist != null && (starImportsBlacklist.contains(packageName)
+                || starImportsBlacklist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith))) {
             throw new SecurityException("Importing [" + packageName + "] is not allowed");
         }
     }
 
     protected void assertImportIsAllowed(final String className) {
-        if (importsWhitelist != null && !importsWhitelist.contains(className)) {
+        if (importsWhitelist != null || starImportsWhitelist != null) {
+            if (importsWhitelist != null && importsWhitelist.contains(className)) {
+                return;
+            }
             if (starImportsWhitelist != null) {
-                // we should now check if the import is in the star imports
-                ClassNode node = ClassHelper.make(className);
-                final String packageName = node.getPackageName();
-                if (!starImportsWhitelist.contains(packageName + ".*")) {
-                    throw new SecurityException("Importing [" + className + "] is not allowed");
+                String packageName = className.substring(0, className.lastIndexOf('.') + 1) + "*";
+                if (starImportsWhitelist.contains(packageName)
+                        || starImportsWhitelist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith)) {
+                    return;
                 }
-            } else {
-                throw new SecurityException("Importing [" + className + "] is not allowed");
             }
-        }
-        if (importsBlacklist != null && importsBlacklist.contains(className)) {
             throw new SecurityException("Importing [" + className + "] is not allowed");
-        }
-        // check that there's no star import blacklist
-        if (starImportsBlacklist != null) {
-            ClassNode node = ClassHelper.make(className);
-            final String packageName = node.getPackageName();
-            if (starImportsBlacklist.contains(packageName + ".*")) {
+        } else {
+            if (importsBlacklist != null && importsBlacklist.contains(className)) {
                 throw new SecurityException("Importing [" + className + "] is not allowed");
             }
+            if (starImportsBlacklist != null) {
+                String packageName = className.substring(0, className.lastIndexOf('.') + 1) + "*";
+                if (starImportsBlacklist.contains(packageName) ||
+                        starImportsBlacklist.stream().filter(it -> it.endsWith(".")).anyMatch(packageName::startsWith)) {
+                    throw new SecurityException("Importing [" + className + "] is not allowed");
+                }
+            }
         }
     }
 
diff --git a/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy b/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
index d4ea67f..2218f9b 100644
--- a/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
+++ b/src/test/org/codehaus/groovy/control/customizers/SecureASTCustomizerTest.groovy
@@ -18,48 +18,49 @@
  */
 package org.codehaus.groovy.control.customizers
 
-import groovy.test.GroovyTestCase
 import org.codehaus.groovy.ast.expr.BinaryExpression
 import org.codehaus.groovy.ast.expr.ConstantExpression
 import org.codehaus.groovy.ast.expr.MethodCallExpression
 import org.codehaus.groovy.control.CompilerConfiguration
 import org.codehaus.groovy.control.MultipleCompilationErrorsException
 import org.codehaus.groovy.syntax.Types
+import org.junit.Before
+import org.junit.Test
 
 /**
- * Tests for the {@link SecureASTCustomizer} class.
+ * Tests for {@link SecureASTCustomizer}.
  */
-class SecureASTCustomizerTest extends GroovyTestCase {
-    CompilerConfiguration configuration
-    SecureASTCustomizer customizer
+final class SecureASTCustomizerTest {
 
+    private final CompilerConfiguration configuration = new CompilerConfiguration()
+    private final SecureASTCustomizer customizer = new SecureASTCustomizer()
+
+    @Before
     void setUp() {
-        configuration = new CompilerConfiguration()
-        customizer = new SecureASTCustomizer()
         configuration.addCompilationCustomizers(customizer)
     }
 
-    private boolean hasSecurityException(Closure closure) {
-        boolean result = false;
+    private static boolean hasSecurityException(Closure closure) {
+        boolean result = false
         try {
             closure()
         } catch (SecurityException e) {
             result = true
         } catch (MultipleCompilationErrorsException e) {
-            result = e.errorCollector.errors.any {it.cause?.class == SecurityException }
+            result = e.errorCollector.errors.any { it.cause?.class == SecurityException }
         }
-
-        result
+        return result
     }
 
+    @Test
     void testPackageDefinition() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             package dummy
             class A {
             }
             new A()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.packageAllowed = false
@@ -68,14 +69,15 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
+    @Test
     void testMethodDefinition() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             def method() {
                 true
             }
             method()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.methodDefinitionAllowed = false
@@ -84,16 +86,17 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
+    @Test
     void testMethodDefinitionInClass() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             class A {
                 def method() {
                     true
                 }
             }
             new A()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.methodDefinitionAllowed = false
@@ -102,328 +105,433 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
+    @Test
     void testClassExtendingClassWithMethods() {
-        def shell = new GroovyShell(configuration)
-        String script = """
+        String script = '''
             class A extends LinkedList {
             }
             new A()
-        """
+        '''
+        def shell = new GroovyShell(configuration)
         shell.evaluate(script)
         // no error means success
         customizer.methodDefinitionAllowed = false
         shell.evaluate(script)
     }
 
+    @Test
     void testExpressionWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.expressionsWhitelist = [BinaryExpression, ConstantExpression]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 class A {}
                 new A()
-            """)
+            ''')
         }
     }
 
+    @Test
     void testExpressionBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.expressionsBlacklist = [MethodCallExpression]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 1+1
                 if (1+1==2) {
                     "test".length()
                 }
-            """)
+            ''')
         }
     }
 
+    @Test
     void testTokenWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.tokensWhitelist = [Types.PLUS, Types.MINUS]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1;1-1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 if (i==2) println 'ok'
-            """)
+            ''')
         }
     }
 
+    @Test
     void testTokenBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.tokensBlacklist = [Types.PLUS_PLUS]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1+1;1-1')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
                 i++
-            """)
+            ''')
         }
     }
 
+    @Test
     void testImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.importsWhitelist = ['java.util.ArrayList']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.LinkedList
-            new LinkedList()
-        """)
+            shell.evaluate('''
+                import java.util.LinkedList
+                new LinkedList()
+            ''')
         }
     }
 
-    void testStarImportWhiteList() {
-        def shell = new GroovyShell(configuration)
+    @Test
+    void testStarImportWhiteList1() {
         customizer.starImportsWhitelist = ['java.util.*']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.concurrent.atomic.AtomicInteger
-            new AtomicInteger(0)
-        """)
+            shell.evaluate('''
+                import java.util.concurrent.atomic.AtomicInteger
+                new AtomicInteger(0)
+            ''')
         }
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.*
-            import java.util.concurrent.atomic.*
-            new ArrayList()
-            new AtomicInteger(0)
-        """)
+            shell.evaluate('''
+                import java.util.*
+                import java.util.concurrent.atomic.*
+                new ArrayList()
+                new AtomicInteger(0)
+            ''')
         }
     }
 
-    void testStarImportWhiteListWithImportWhiteList() {
+    @Test
+    void testStarImportWhiteList2() {
+        customizer.starImportsWhitelist = ['java.**']
         def shell = new GroovyShell(configuration)
+        shell.evaluate('''
+            import java.lang.Object
+            Object obj
+        ''')
+        assert hasSecurityException {
+            shell.evaluate('''
+                import javax.swing.Action
+                Action act
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.util.*
+                import javax.swing.*
+                Object obj
+                Action act
+            ''')
+        }
+    }
+
+    @Test
+    void testStarImportWhiteListWithImportWhiteList() {
         customizer.importsWhitelist = ['java.util.concurrent.atomic.AtomicInteger']
         customizer.starImportsWhitelist = ['java.util.*']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
-        shell.evaluate("""
+        ''')
+        shell.evaluate('''
             import java.util.concurrent.atomic.AtomicInteger
             new AtomicInteger(0)
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import java.util.concurrent.atomic.AtomicBoolean
-            new AtomicBoolean(false)
-        """)
+            shell.evaluate('''
+                import java.util.concurrent.atomic.AtomicBoolean
+                new AtomicBoolean(false)
+            ''')
         }
     }
 
+    @Test
     void testImportBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.importsBlacklist = ['java.util.LinkedList']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
+            shell.evaluate('''
+                import java.util.LinkedList
+                new LinkedList()
+            ''')
+        }
+    }
+
+    @Test
+    void testStarImportBlackList1() {
+        customizer.starImportsBlacklist = ['java.lang.*']
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.LinkedList
-            new LinkedList()
-        """)
+            import javax.swing.Action
+            LinkedList list
+            Action act
+        ''')
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.lang.Object
+                Object obj
+            ''')
         }
     }
 
-    void testStarImportBlackListWithImportBlackList() {
+    @Test
+    void testStarImportBlackList2() {
+        customizer.starImportsBlacklist = ['java.**']
         def shell = new GroovyShell(configuration)
+        shell.evaluate('''
+            import javax.swing.Action
+            Action act
+        ''')
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.lang.Object
+                Object obj
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                import java.util.Deque
+                Deque deck
+            ''')
+        }
+    }
+
+    @Test
+    void testStarImportBlackListWithImportBlackList() {
         customizer.importsBlacklist = ['java.util.concurrent.atomic.AtomicBoolean']
         customizer.starImportsBlacklist = ['java.util.*']
+        def shell = new GroovyShell(configuration)
         assert hasSecurityException {
-            shell.evaluate("""
-                 import java.util.ArrayList
-                 new ArrayList()
-             """)
+            shell.evaluate('''
+                import java.util.ArrayList
+                new ArrayList()
+            ''')
         }
-        shell.evaluate("""
-             import java.util.concurrent.atomic.AtomicInteger
-             new AtomicInteger(0)
-         """)
+        shell.evaluate('''
+            import java.util.concurrent.atomic.AtomicInteger
+            new AtomicInteger(0)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-             import java.util.concurrent.atomic.AtomicBoolean
-             new AtomicBoolean(false)
-         """)
+            shell.evaluate('''
+                import java.util.concurrent.atomic.AtomicBoolean
+                new AtomicBoolean(false)
+            ''')
         }
     }
 
-
+    @Test
     void testIndirectImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.importsWhitelist = ['java.util.ArrayList']
         customizer.indirectImportCheckEnabled = true
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""            
-            new java.util.LinkedList()
-        """)
-
-            assert hasSecurityException {
-                shell.evaluate("""
-            return java.util.LinkedList.&size
-        """)
-            }
+            shell.evaluate('''
+                new java.util.LinkedList()
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                return java.util.LinkedList.&size
+            ''')
         }
     }
 
+    @Test
     void testIndirectStarImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.starImportsWhitelist = ['java.util.*']
         customizer.indirectImportCheckEnabled = true
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import java.util.ArrayList
             new ArrayList()
-        """)
-        shell.evaluate("""
+        ''')
+        shell.evaluate('''
             new java.util.ArrayList()
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            new java.util.concurrent.atomic.AtomicBoolean(false)
-        """)
-
-            assert hasSecurityException {
-                shell.evaluate("""
-            return java.util.concurrent.atomic.AtomicBoolean.&get
-        """)
-            }
+            shell.evaluate('''
+                new java.util.concurrent.atomic.AtomicBoolean(false)
+            ''')
+        }
+        assert hasSecurityException {
+            shell.evaluate('''
+                return java.util.concurrent.atomic.AtomicBoolean.&get
+            ''')
         }
     }
 
+    @Test
     void testStaticImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.staticImportsWhitelist = ['java.lang.Math.PI']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import static java.lang.Math.PI
             PI
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import static java.lang.Math.PI
-            import static java.lang.Math.cos
-            cos(PI)
-        """)
+            shell.evaluate('''
+                import static java.lang.Math.PI
+                import static java.lang.Math.cos
+                cos(PI)
+            ''')
         }
     }
 
+    @Test
     void testStaticStarImportWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.staticStarImportsWhitelist = ['java.lang.Math.*']
-        shell.evaluate("""
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('''
             import static java.lang.Math.PI
             import static java.lang.Math.cos
             cos(PI)
-        """)
+        ''')
         assert hasSecurityException {
-            shell.evaluate("""
-            import static java.util.Collections.*
-            sort([5,4,2])
-        """)
+            shell.evaluate('''
+                import static java.util.Collections.*
+                sort([5,4,2])
+            ''')
         }
     }
 
+    @Test
     void testIndirectStaticImport() {
-        def shell = new GroovyShell(configuration)
         customizer.staticImportsWhitelist = ['java.lang.Math.PI']
         customizer.indirectImportCheckEnabled = true
-        assert hasSecurityException {shell.evaluate('java.lang.Math.cos(1)')}
+        def shell = new GroovyShell(configuration)
+        assert hasSecurityException {
+            shell.evaluate('java.lang.Math.cos(1)')
+        }
     }
 
+    @Test
     void testIndirectStaticStarImport() {
-        def shell = new GroovyShell(configuration)
         customizer.staticStarImportsWhitelist = ['java.lang.Math.*']
         customizer.indirectImportCheckEnabled = true
+        def shell = new GroovyShell(configuration)
         shell.evaluate('java.lang.Math.cos(1)')
-        assert hasSecurityException {shell.evaluate('java.util.Collections.unmodifiableList([1])')}
+        assert hasSecurityException {
+            shell.evaluate('java.util.Collections.unmodifiableList([1])')
+        }
     }
 
+    @Test
     void testConstantTypeWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.constantTypesClassesWhiteList = [Integer.TYPE]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1')
-        assert hasSecurityException {shell.evaluate('"string"')}
-        assert hasSecurityException {shell.evaluate('2d')}
+        assert hasSecurityException {
+            shell.evaluate('"string"')
+        }
+        assert hasSecurityException {
+            shell.evaluate('2d')
+        }
     }
 
+    @Test
     void testConstantTypeBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.constantTypesClassesBlackList = [String]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1')
         shell.evaluate('2d')
-        assert hasSecurityException {shell.evaluate('"string"')}
+        assert hasSecurityException {
+            shell.evaluate('"string"')
+        }
     }
 
+    @Test
     void testReceiverWhiteList() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesWhiteList = [Integer.TYPE]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
-        assert hasSecurityException {shell.evaluate('"string".toUpperCase()')}
-        assert hasSecurityException {shell.evaluate('2.0.multiply(4)')}
+        assert hasSecurityException {
+            shell.evaluate('"string".toUpperCase()')
+        }
+        assert hasSecurityException {
+            shell.evaluate('2.0.multiply(4)')
+        }
     }
 
+    @Test
     void testReceiverBlackList() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesBlackList = [String]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
         shell.evaluate('2.0.multiply(4)')
-        assert hasSecurityException {shell.evaluate('"string".toUpperCase()')}
+        assert hasSecurityException {
+            shell.evaluate('"string".toUpperCase()')
+        }
     }
 
+    @Test
     void testReceiverWhiteListWithStaticMethod() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesWhiteList = [Integer.TYPE]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
-        assert hasSecurityException {shell.evaluate('java.lang.Math.cos(2)')}
+        assert hasSecurityException {
+            shell.evaluate('java.lang.Math.cos(2)')
+        }
     }
 
+    @Test
     void testReceiverBlackListWithStaticMethod() {
-        def shell = new GroovyShell(configuration)
         customizer.receiversClassesBlackList = [Math]
+        def shell = new GroovyShell(configuration)
         shell.evaluate('1.plus(1)')
         shell.evaluate('Collections.sort([])')
-        assert hasSecurityException {shell.evaluate('java.lang.Math.cos(2)')}
+        assert hasSecurityException {
+            shell.evaluate('java.lang.Math.cos(2)')
+        }
     }
 
-    // Testcase for GROOVY-4978
+    @Test // GROOVY-4978
     void testVisitMethodBody() {
-        def shell = new GroovyShell(configuration)
         customizer.importsBlacklist = [
                 "java.lang.System",
                 "groovy.lang.GroovyShell",
                 "groovy.lang.GroovyClassLoader"]
         customizer.indirectImportCheckEnabled = true
-
-        assert hasSecurityException {shell.evaluate('System.println(1)')}
-        assert hasSecurityException {shell.evaluate('def x() { System.println(1) }')}
+        def shell = new GroovyShell(configuration)
+        assert hasSecurityException {
+            shell.evaluate('System.println(1)')
+        }
+        assert hasSecurityException {
+            shell.evaluate('def x() { System.println(1) }')
+        }
     }
 
-    // GROOVY-7424
+    @Test // GROOVY-7424
     void testClassWithInterfaceVisitable() {
         def shell = new GroovyShell(configuration)
         shell.evaluate '''
             interface Foo { def baz() }
-            class Bar implements Foo { def baz() { 42 }}
+            class Bar implements Foo { def baz() { 42 } }
             assert new Bar().baz() == 42
         '''
     }
 
-    // GROOVY-6153
+    @Test // GROOVY-6153
     void testDeterministicWhitelistBehaviour() {
-        def shell = new GroovyShell(configuration)
         def classWhiteList = ["java.lang.Object", "test"]
         customizer.with {
             setIndirectImportCheckEnabled(true);
@@ -433,7 +541,7 @@ class SecureASTCustomizerTest extends GroovyTestCase {
             setClosuresAllowed(true);
             setMethodDefinitionAllowed(true);
         }
-
+        def shell = new GroovyShell(configuration)
         assert hasSecurityException {
             shell.evaluate '''
                 java.lang.System.out.println("run ")
@@ -441,9 +549,8 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
-    // GROOVY-6153
+    @Test // GROOVY-6153
     void testDeterministicWhitelistBehaviour2() {
-        def shell = new GroovyShell(configuration)
         def classWhiteList = ["java.lang.Object", "test"]
         customizer.with {
             setIndirectImportCheckEnabled(true);
@@ -453,7 +560,7 @@ class SecureASTCustomizerTest extends GroovyTestCase {
             setClosuresAllowed(true);
             setMethodDefinitionAllowed(true);
         }
-
+        def shell = new GroovyShell(configuration)
         assert hasSecurityException {
             shell.evaluate '''
                 java.lang.Long x = 666L
@@ -461,24 +568,14 @@ class SecureASTCustomizerTest extends GroovyTestCase {
         }
     }
 
-    // GROOVY-8135
+    @Test // GROOVY-8135
     void testStarImportsWhiteListWithIndirectImportCheckEnabled() {
-        SecureASTCustomizer customizer = new SecureASTCustomizer()
-        customizer.setIndirectImportCheckEnabled(true)
-
-        List<String> starImportsWhitelist = new ArrayList<String>()
-        starImportsWhitelist.add("java.lang")
-        customizer.setStarImportsWhitelist(starImportsWhitelist)
-
-        CompilerConfiguration cc = new CompilerConfiguration()
-        cc.addCompilationCustomizers(customizer)
-
-        ClassLoader parent = getClass().getClassLoader()
-        GroovyClassLoader loader = new GroovyClassLoader(parent, cc)
-        loader.parseClass("Object object = new Object()")
-        loader.parseClass("Object object = new Object(); object.hashCode()")
-        loader.parseClass("Object[] array = new Object[0]; array.size()")
-        loader.parseClass("Object[][] array = new Object[0][0]; array.size()")
+        customizer.indirectImportCheckEnabled = true
+        customizer.starImportsWhitelist = ['java.lang']
+        def shell = new GroovyShell(configuration)
+        shell.evaluate('Object object = new Object()')
+        shell.evaluate('Object object = new Object(); object.hashCode()')
+        shell.evaluate('Object[] array = new Object[0]; array.size()')
+        shell.evaluate('Object[][] array = new Object[0][0]; array.size()')
     }
-
 }