defaultJvmOpts argument quoting for scripts

gradle · May 2, 2013 · a7a8fec · a7a8fec
1 parent 15ee375
commit a7a8fec
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 5 deletions.
diff --git a/...jects/plugins/src/main/groovy/org/gradle/api/internal/plugins/StartScriptGenerator.groovy b/...jects/plugins/src/main/groovy/org/gradle/api/internal/plugins/StartScriptGenerator.groovy
@@ -66,7 +66,17 @@ class StartScriptGenerator {
 
     String generateUnixScriptContent() {
         def unixClassPath = classpath.collect { "\$APP_HOME/${it.replace('\\', '/')}" }.join(":")
-        def defaultJvmOptsString = defaultJvmOpts.collect { it.replace(' ', '\\ ') }.join(' ')
+        def quotedDefaultJvmOpts = defaultJvmOpts.collect{
+            //quote ', ", \, $. Probably not perfect. TODO: identify non-working cases, fail-fast on them
+            it = it.replace('\\', '\\\\')
+            it = it.replace('"', '\\"')
+            it = it.replace(/'/, /'"'"'/)
+            it = it.replace('$', '\\$')
+            (/"${it}"/)
+        }
+        //put the whole arguments string in single quotes, unless defaultJvmOpts was empty,
+        // in which case we output "" to stay compatible with existing builds that scan the script for it
+        def defaultJvmOptsString = (quotedDefaultJvmOpts ? /'${quotedDefaultJvmOpts.join(' ')}'/ : '""')
         def binding = [applicationName: applicationName,
                 optsEnvironmentVar: optsEnvironmentVar,
                 mainClassName: mainClassName,
@@ -85,7 +95,27 @@ class StartScriptGenerator {
     String generateWindowsScriptContent() {
         def windowsClassPath = classpath.collect { "%APP_HOME%\\${it.replace('/', '\\')}" }.join(";")
         def appHome = appHomeRelativePath.replace('/', '\\')
-        def defaultJvmOptsString = defaultJvmOpts.collect { it.replace(' ', '\\ ') }.join(' ')
+        //argument quoting:
+        // - " must be encoded as \"
+        // - % must be encoded as %%
+        // - pathological case: \" must be encoded as \\\", but other than that, \ MUST NOT be quoted
+        // - other characters (including ') will not be quoted
+        // - use a state machine rather than regexps
+        def quotedDefaultJvmOpts = defaultJvmOpts.collect {
+            def wasOnBackslash = false
+            it = it.collect { ch ->
+                def repl = ch
+                if (ch == '%') {
+                    repl = '%%'
+                } else if (ch == '"') {
+                    repl = (wasOnBackslash ? '\\' : '') + '\\"'
+                }
+                wasOnBackslash = (ch == '\\')
+                repl
+            }
+            (/"${it.join()}"/)
+        }
+        def defaultJvmOptsString = quotedDefaultJvmOpts.join(' ')
         def binding = [applicationName: applicationName,
                 optsEnvironmentVar: optsEnvironmentVar,
                 exitEnvironmentVar: exitEnvironmentVar,

diff --git a/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt b/subprojects/plugins/src/main/resources/org/gradle/api/internal/plugins/unixStartScript.txt
@@ -7,7 +7,7 @@
 ##############################################################################
 
 # Add default JVM options here. You can also use JAVA_OPTS and ${optsEnvironmentVar} to pass JVM options to this script.
-DEFAULT_JVM_OPTS="${defaultJvmOpts}"
+DEFAULT_JVM_OPTS=${defaultJvmOpts}
 
 APP_NAME="${applicationName}"
 APP_BASE_NAME=`basename "\$0"`

diff --git a/...s/plugins/src/test/groovy/org/gradle/api/internal/plugins/StartScriptGeneratorTest.groovy b/...s/plugins/src/test/groovy/org/gradle/api/internal/plugins/StartScriptGeneratorTest.groovy
@@ -78,7 +78,37 @@ class StartScriptGeneratorTest extends Specification {
         when:
         String windowsScriptContent = generator.generateWindowsScriptContent()
         then:
-        windowsScriptContent.contains("set DEFAULT_JVM_OPTS=-Dfoo=bar -Xint")
+        windowsScriptContent.contains('set DEFAULT_JVM_OPTS="-Dfoo=bar" "-Xint"')
+    }
+
+    def "defaultJvmOpts is expanded properly in windows script -- spaces"() {
+        given:
+        generator.defaultJvmOpts = ['-Dfoo=bar baz', '-Xint']
+        generator.scriptRelPath = "bin"
+        when:
+        String windowsScriptContent = generator.generateWindowsScriptContent()
+        then:
+        windowsScriptContent.contains(/set DEFAULT_JVM_OPTS="-Dfoo=bar baz" "-Xint"/)
+    }
+
+    def "defaultJvmOpts is expanded properly in windows script -- double quotes"() {
+        given:
+        generator.defaultJvmOpts = ['-Dfoo=b"ar baz', '-Xi""nt', '-Xpatho\\"logical']
+        generator.scriptRelPath = "bin"
+        when:
+        String windowsScriptContent = generator.generateWindowsScriptContent()
+        then:
+        windowsScriptContent.contains(/set DEFAULT_JVM_OPTS="-Dfoo=b\"ar baz" "-Xi\"\"nt" "-Xpatho\\\"logical"/)
+    }
+
+    def "defaultJvmOpts is expanded properly in windows script -- backslashes and shell metacharacters"() {
+        given:
+        generator.defaultJvmOpts = ['-Dfoo=b\\ar baz', '-Xint%PATH%']
+        generator.scriptRelPath = "bin"
+        when:
+        String windowsScriptContent = generator.generateWindowsScriptContent()
+        then:
+        windowsScriptContent.contains(/set DEFAULT_JVM_OPTS="-Dfoo=b\ar baz" "-Xint%%PATH%%"/)
     }
 
     def "defaultJvmOpts is expanded properly in unix script"() {
@@ -88,6 +118,55 @@ class StartScriptGeneratorTest extends Specification {
         when:
         String unixScriptContent = generator.generateUnixScriptContent()
         then:
-        unixScriptContent.contains('DEFAULT_JVM_OPTS="-Dfoo=bar -Xint"')
+        unixScriptContent.contains('DEFAULT_JVM_OPTS=\'"-Dfoo=bar" "-Xint"\'')
+    }
+
+    def "defaultJvmOpts is expanded properly in unix script -- spaces"() {
+        given:
+        generator.defaultJvmOpts = ['-Dfoo=bar baz', '-Xint']
+        generator.scriptRelPath = "bin"
+        when:
+        String unixScriptContent = generator.generateUnixScriptContent()
+        then:
+        unixScriptContent.contains(/DEFAULT_JVM_OPTS='"-Dfoo=bar baz" "-Xint"'/)
+    }
+
+    def "defaultJvmOpts is expanded properly in unix script -- double quotes"() {
+        given:
+        generator.defaultJvmOpts = ['-Dfoo=b"ar baz', '-Xi""nt']
+        generator.scriptRelPath = "bin"
+        when:
+        String unixScriptContent = generator.generateUnixScriptContent()
+        then:
+        unixScriptContent.contains(/DEFAULT_JVM_OPTS='"-Dfoo=b\"ar baz" "-Xi\"\"nt"'/)
+    }
+
+    def "defaultJvmOpts is expanded properly in unix script -- single quotes"() {
+        given:
+        generator.defaultJvmOpts = ['-Dfoo=b\'ar baz', '-Xi\'\'nt']
+        generator.scriptRelPath = "bin"
+        when:
+        String unixScriptContent = generator.generateUnixScriptContent()
+        then:
+        unixScriptContent.contains(/DEFAULT_JVM_OPTS='"-Dfoo=b'"'"'ar baz" "-Xi'"'"''"'"'nt"'/)
+    }
+
+    def "defaultJvmOpts is expanded properly in unix script -- backslashes and shell metacharacters"() {
+        given:
+        generator.defaultJvmOpts = ['-Dfoo=b\\ar baz', '-Xint$PATH']
+        generator.scriptRelPath = "bin"
+        when:
+        String unixScriptContent = generator.generateUnixScriptContent()
+        then:
+        unixScriptContent.contains(/DEFAULT_JVM_OPTS='"-Dfoo=b\\ar baz" "-Xint/ + '\\$PATH' + /"'/)
+    }
+
+    def "defaultJvmOpts is expanded properly in unix script -- empty list"() {
+        given:
+        generator.scriptRelPath = "bin"
+        when:
+        String unixScriptContent = generator.generateUnixScriptContent()
+        then:
+        unixScriptContent.contains(/DEFAULT_JVM_OPTS=""/)
     }
 }