You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2018/11/04 15:43:48 UTC

[3/3] groovy git commit: Abandon evaluating script to avoid security issues

Abandon evaluating script to avoid security issues


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/1d20e307
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/1d20e307
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/1d20e307

Branch: refs/heads/master
Commit: 1d20e307251aedde4f73abe879e2b548f2d16a17
Parents: 8e24158
Author: Daniel Sun <su...@apache.org>
Authored: Sun Nov 4 22:07:32 2018 +0800
Committer: Daniel Sun <su...@apache.org>
Committed: Sun Nov 4 23:43:06 2018 +0800

----------------------------------------------------------------------
 src/main/groovy/groovy/util/GProperties.groovy  | 66 +++++---------------
 .../groovy/util/gproperties.properties          |  6 +-
 .../groovy/util/gproperties_import2.properties  |  2 +-
 src/test/groovy/util/GPropertiesTest.groovy     | 38 ++---------
 4 files changed, 21 insertions(+), 91 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/1d20e307/src/main/groovy/groovy/util/GProperties.groovy
----------------------------------------------------------------------
diff --git a/src/main/groovy/groovy/util/GProperties.groovy b/src/main/groovy/groovy/util/GProperties.groovy
index 500baf5..fe42982 100644
--- a/src/main/groovy/groovy/util/GProperties.groovy
+++ b/src/main/groovy/groovy/util/GProperties.groovy
@@ -57,26 +57,15 @@ import java.util.regex.Pattern
  *      assert 'Hello,Daniel' == gp.getProperty('greeting.daniel')
  * </pre>
  *
- * 3) Evaluating with ${...}, e.g.
+ * 3) Escaping with {{...}}, e.g.
  * <pre>
  *      # gproperties.properties
- *      greeting.daniel=Hello,Daniel in ${new java.text.SimpleDateFormat('yyyyMMdd').format(new Date())}
+ *      greeting.daniel={{groovy.greeting}},{{some.name}}
  *
  *      // groovy script
- *      def gp = new GProperties(true) // Note: we should enable evaluating script manually
- *      gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
- *      assert 'Hello,Daniel in 2018' == gp.getProperty('greeting.daniel') // Given running the script in 2018
- * </pre>
- *
- * 4) Escaping with {{...}}, ${{...}}, e.g.
- * <pre>
- *      # gproperties.properties
- *      greeting.daniel={{groovy.greeting}},{{some.name}} in ${{new java.text.SimpleDateFormat('yyyyMMdd').format(new Date())}}
- *
- *      // groovy script
- *      def gp = new GProperties(true)
+ *      def gp = new GProperties()
  *      gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
- *      assert '{groovy.greeting},{some.name} in ${new java.text.SimpleDateFormat('yyyyMMdd').format(new Date())}' == gp.getProperty('greeting.daniel')
+ *      assert '{groovy.greeting},{some.name}' == gp.getProperty('greeting.daniel')
  * </pre>
  *
  * @since 3.0.0
@@ -85,63 +74,46 @@ import java.util.regex.Pattern
 class GProperties extends Properties {
     private static final long serialVersionUID = 6112578636029876860L
     public static final String IMPORT_PROPERTIES_KEY = 'import.properties'
-    public static final String VAR_KEY = 'key'
-    public static final String VAR_PROPERTIES = 'properties'
-    private static final Pattern GROOVY_SCRIPT_PATTERN = Pattern.compile(/[$][{]\s*(.+?)\s*[}]/)
-    private static final Pattern INTERPOLATE_PATTERN = Pattern.compile(/[{]\s*(.+?)\s*[}]/)
+    private static final Pattern INTERPOLATE_PATTERN = Pattern.compile(/[{](.+?)[}]/)
     private static final Pattern ESCAPE_PATTERN = Pattern.compile(/[{]([{].+?[}])[}]/)
     private static final String LEFT_CURLY_BRACE = '{'
     private static final String RIGHT_CURLY_BRACE = '}'
     private static final String COMMA = ','
-    private final boolean evaluateScriptEnabled
-    private final GroovyShell groovyShell
-    private final List<GProperties> importPropertiesList = new ArrayList<>()
+    private final List<GProperties> importPropertiesList = new LinkedList<>()
 
-    GProperties(boolean evaluateScriptEnabled=false, GroovyShell groovyShell=new GroovyShell()) {
-        this(null, evaluateScriptEnabled, groovyShell)
+    GProperties() {
+        this(null)
     }
 
-    GProperties(Properties defaults, boolean evaluateScriptEnabled=false, GroovyShell groovyShell=new GroovyShell()) {
+    GProperties(Properties defaults) {
         super(defaults)
-        this.evaluateScriptEnabled = evaluateScriptEnabled
-        this.groovyShell = groovyShell
     }
 
     @Override
     String getProperty(String key) {
         String value = super.getProperty(key)
 
-        if (!value) {
+        if (null == value) {
             for (GProperties importProperties : importPropertiesList) {
                 value = importProperties.getProperty(key)
 
-                if (value) {
+                if (null != value) {
                     break
                 }
             }
         }
 
-        if (!value) {
+        if (null == value) {
             return value
         }
 
-        if (evaluateScriptEnabled) {
-            value = value.replaceAll(GROOVY_SCRIPT_PATTERN) { String _0, String _1 ->
-                if (_1.startsWith(LEFT_CURLY_BRACE) && _1.endsWith(RIGHT_CURLY_BRACE)) {
-                    return _0
-                }
-                processImplicitVariables(key) {
-                    groovyShell.evaluate(_1)
-                }
-            }
-        }
-
         value = value.replaceAll(INTERPOLATE_PATTERN) { String _0, String _1 ->
             if (_1.startsWith(LEFT_CURLY_BRACE) && _1.endsWith(RIGHT_CURLY_BRACE)) {
                 return _0
             }
 
-            this.getProperty(_1) ?: _0
+            def p = this.getProperty(_1.trim())
+            null == p ? _0 : p
         }
 
         value.replaceAll(ESCAPE_PATTERN) { String _0, String _1 ->
@@ -191,14 +163,4 @@ class GProperties extends Properties {
             importPropertiesList << importProperties
         }
     }
-
-    private synchronized Object processImplicitVariables(String key, Closure c) {
-        groovyShell.setVariable(VAR_KEY, key)
-        groovyShell.setVariable(VAR_PROPERTIES, this)
-        def v = c()
-        groovyShell.removeVariable(VAR_PROPERTIES)
-        groovyShell.removeVariable(VAR_KEY)
-
-        return v
-    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/1d20e307/src/test-resources/groovy/util/gproperties.properties
----------------------------------------------------------------------
diff --git a/src/test-resources/groovy/util/gproperties.properties b/src/test-resources/groovy/util/gproperties.properties
index e4f488d..f7f7db3 100644
--- a/src/test-resources/groovy/util/gproperties.properties
+++ b/src/test-resources/groovy/util/gproperties.properties
@@ -20,9 +20,5 @@
 import.properties=/groovy/util/gproperties_import.properties,/groovy/util/gproperties_import3.properties
 groovy.greeting={greeting.word},{some.name}
 groovy.greeting.with.smile={groovy.greeting} :)
-groovy.greeting.with.time={groovy.greeting.with.smile} in ${new java.text.SimpleDateFormat('yyyyMMdd').format(new Date())}
 groovy.greeting.with.missing=Hello,{none} {0}
-groovy.greeting.with.script=Hello,${properties.getProperty('some.name')}
-groovy.greeting.with.key=Hello,${key}
-groovy.greeting.with.escapes=Hello,{{some.name}}
-groovy.greeting.with.escapes2=Hello,${{properties.getProperty('some.name')}}
\ No newline at end of file
+groovy.greeting.with.escapes=Hello,{{some.name}}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/groovy/blob/1d20e307/src/test-resources/groovy/util/gproperties_import2.properties
----------------------------------------------------------------------
diff --git a/src/test-resources/groovy/util/gproperties_import2.properties b/src/test-resources/groovy/util/gproperties_import2.properties
index 7d41c9f..99a9f7a 100644
--- a/src/test-resources/groovy/util/gproperties_import2.properties
+++ b/src/test-resources/groovy/util/gproperties_import2.properties
@@ -17,4 +17,4 @@
 #  under the License.
 #
 
-greeting.word=H${'ell'}o
+greeting.word=Hello

http://git-wip-us.apache.org/repos/asf/groovy/blob/1d20e307/src/test/groovy/util/GPropertiesTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/util/GPropertiesTest.groovy b/src/test/groovy/util/GPropertiesTest.groovy
index fe9ae0d..8967d66 100644
--- a/src/test/groovy/util/GPropertiesTest.groovy
+++ b/src/test/groovy/util/GPropertiesTest.groovy
@@ -20,7 +20,7 @@ package groovy.util
 
 class GPropertiesTest extends GroovyTestCase {
     void testImportProperties() {
-        def gp = new GProperties(true)
+        def gp = new GProperties()
         gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
 
         assert '/groovy/util/gproperties_import.properties,/groovy/util/gproperties_import3.properties' == gp.getProperty('import.properties')
@@ -30,14 +30,14 @@ class GPropertiesTest extends GroovyTestCase {
     }
 
     void testInterpolate() {
-        def gp = new GProperties(true)
+        def gp = new GProperties()
         gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
 
         assert 'Hello,Daniel' == gp.getProperty('groovy.greeting')
     }
 
     void testInterpolate2() {
-        def gp = new GProperties(true)
+        def gp = new GProperties()
         gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
 
         assert 'Hello,Daniel :)' == gp.getProperty('groovy.greeting.with.smile')
@@ -51,7 +51,7 @@ class GPropertiesTest extends GroovyTestCase {
     }
 
     void testInterpolate4() {
-        def gp = new GProperties(true)
+        def gp = new GProperties()
         gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
 
         assert 'Hello,Daniel' == gp.getProperty('greeting.daniel')
@@ -61,7 +61,7 @@ class GPropertiesTest extends GroovyTestCase {
         def gp = new GProperties()
         gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
 
-        assert '''H${'ell'}o,Daniel''' == gp.getProperty('greeting.daniel')
+        assert '''Hello,Daniel''' == gp.getProperty('greeting.daniel')
     }
 
     void testEscape() {
@@ -70,32 +70,4 @@ class GPropertiesTest extends GroovyTestCase {
 
         assert 'Hello,{some.name}' == gp.getProperty('groovy.greeting.with.escapes')
     }
-
-    void testEscape2() {
-        def gp = new GProperties()
-        gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
-
-        assert '''Hello,${properties.getProperty('some.name')}''' == gp.getProperty('groovy.greeting.with.escapes2')
-    }
-
-    void testInterpolateGroovyScript() {
-        def gp = new GProperties(true)
-        gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
-
-        assert "Hello,Daniel :) in ${new java.text.SimpleDateFormat('yyyyMMdd').format(new Date())}" == gp.getProperty('groovy.greeting.with.time')
-    }
-
-    void testInterpolateGroovyScript2() {
-        def gp = new GProperties(true)
-        gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
-
-        assert "Hello,Daniel" == gp.getProperty('groovy.greeting.with.script')
-    }
-
-    void testInterpolateGroovyScript3() {
-        def gp = new GProperties(true)
-        gp.load(GPropertiesTest.getResourceAsStream('/groovy/util/gproperties.properties'))
-
-        assert "Hello,groovy.greeting.with.key" == gp.getProperty('groovy.greeting.with.key')
-    }
 }