Drop custom parameter escaping

Escaping done by `addKeyValuePairs` is already enough. [FIXED JENKINS-42573]
jenkinsci · Mar 10, 2017 · 7752876 · 7752876 · spac3hit · Mar 20, 2017
1 parent e2a5c24
commit 7752876
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 29 deletions.
diff --git a/README.md b/README.md
@@ -10,6 +10,7 @@ In order to release this plugin have a look at [here](RELEASING.md).
 
 ## Release Notes
 * 1.27 (unreleased)
+  * Job parameters are now correctly quoted when passed as system properties [JENKINS-42573](https://issues.jenkins-ci.org/browse/JENKINS-42573)
   * Make finding wrapper location more robust on Windows
   * Increase required core version to 1.642.1
 * 1.26 (Feb 13 2016)

diff --git a/src/main/java/hudson/plugins/gradle/Gradle.java b/src/main/java/hudson/plugins/gradle/Gradle.java
@@ -26,7 +26,6 @@
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -274,7 +273,7 @@ private boolean performTask(boolean dryRun, AbstractBuild<?, ?> build, Launcher
 
 
         Set<String> sensitiveVars = build.getSensitiveBuildVariables();
-        args.addKeyValuePairs(passPropertyOption(), fixParameters(build.getBuildVariables()), sensitiveVars);
+        args.addKeyValuePairs(passPropertyOption(), build.getBuildVariables(), sensitiveVars);
         args.addTokenized(normalizedSwitches);
         args.addTokenized(normalizedTasks);
         if (buildFile != null && buildFile.trim().length() != 0) {
@@ -363,29 +362,6 @@ private String passPropertyOption() {
         return passAsProperties ? "-P" : "-D";
     }
 
-    private Map<String, String> fixParameters(Map<String, String> parmas) {
-        Map<String, String> result = new HashMap<String, String>();
-        for (Map.Entry<String, String> entry : parmas.entrySet()) {
-            String value = entry.getValue();
-            if (isValue2Escape(value)) {
-                result.put(entry.getKey(), "\"" + value + "\"");
-            } else {
-                result.put(entry.getKey(), value);
-            }
-        }
-        return result;
-    }
-
-    private boolean isValue2Escape(String value) {
-        if (value == null) {
-            return false;
-        }
-        if (value.trim().length() == 0) {
-            return false;
-        }
-        return value.contains("<") || value.contains(">");
-    }
-
     @Override
     public DescriptorImpl getDescriptor() {
         return (DescriptorImpl) super.getDescriptor();

diff --git a/src/test/groovy/hudson/plugins/gradle/GradlePluginIntegrationTest.groovy b/src/test/groovy/hudson/plugins/gradle/GradlePluginIntegrationTest.groovy
@@ -188,7 +188,7 @@ task hello << { println 'Hello' }"""))
         'build/some/build.gradle' | [rootBuildScriptDir: 'build']                                                               | [null]
     }
 
-    def "Can use > signs in system properties"() {
+    def "Can use '#propertyValue' in system properties"() {
         given:
         gradleInstallationRule.addInstallation()
         def p = j.createFreeStyleProject()
@@ -198,11 +198,20 @@ task hello << { println 'Hello' }"""))
         p.buildersList.add(new Gradle(tasks: 'printParam', useWrapper: true, useWorkspaceAsHome: true))
 
         when:
-        def build = j.assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause(), new ParametersAction(new TextParameterValue("PARAM", "a < b"))))
+        def build = j.assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause(), new ParametersAction(new TextParameterValue("PARAM", propertyValue))))
 
         then:
-        // TODO: Make this return 'property=a < b'
-        getLog(build).contains('property="a < b"')
+        getLog(build).contains("property=${propertyValue}")
+
+        where:
+        propertyValue << [
+                'a < b',
+                '<foo> <bar/> </foo>',
+                'renaming XYZ >> \'xyz\'',
+                'renaming XYZ >>> \'xyz\'',
+                'renaming XYZ >> "xyz"',
+                'renaming \'XYZ >> \'x"y"z\'"'
+        ]
     }
 
     def "Config roundtrip"() {