[JENKINS-28790] solved the issue masked parameters not masked in wind…

…ows system. toWindowsCommand joins all args in one, missing masked. (cherry picked from commit b9a72bc)
jenkinsci · Mar 30, 2016 · 0782aa5 · 0782aa5
1 parent 951729c
commit 0782aa5
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 29 deletions.
diff --git a/core/src/main/java/hudson/util/ArgumentListBuilder.java b/core/src/main/java/hudson/util/ArgumentListBuilder.java
@@ -314,42 +314,46 @@ public String toStringWithQuote() {
      * @since 1.386
      */
     public ArgumentListBuilder toWindowsCommand(boolean escapeVars) {
-        StringBuilder quotedArgs = new StringBuilder();
+    	ArgumentListBuilder windowsCommand = new ArgumentListBuilder().add("cmd.exe", "/C");
         boolean quoted, percent;
-        for (String arg : args) {
+        for (int i = 0; i < args.size(); i++) {
+        	StringBuilder quotedArgs = new StringBuilder();
+        	String arg = args.get(i);
             quoted = percent = false;
-            for (int i = 0; i < arg.length(); i++) {
-                char c = arg.charAt(i);
+            for (int j = 0; j < arg.length(); j++) {
+                char c = arg.charAt(j);
                 if (!quoted && (c == ' ' || c == '*' || c == '?' || c == ',' || c == ';')) {
-                    quoted = startQuoting(quotedArgs, arg, i);
+                    quoted = startQuoting(quotedArgs, arg, j);
                 }
                 else if (c == '^' || c == '&' || c == '<' || c == '>' || c == '|') {
-                    if (!quoted) quoted = startQuoting(quotedArgs, arg, i);
+                    if (!quoted) quoted = startQuoting(quotedArgs, arg, j);
                     // quotedArgs.append('^'); See note in javadoc above
                 }
                 else if (c == '"') {
-                    if (!quoted) quoted = startQuoting(quotedArgs, arg, i);
+                    if (!quoted) quoted = startQuoting(quotedArgs, arg, j);
                     quotedArgs.append('"');
                 }
                 else if (percent && escapeVars
                          && ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'))) {
-                    if (!quoted) quoted = startQuoting(quotedArgs, arg, i);
+                    if (!quoted) quoted = startQuoting(quotedArgs, arg, j);
                     quotedArgs.append('"').append(c);
                     c = '"';
                 }
                 percent = (c == '%');
                 if (quoted) quotedArgs.append(c);
             }
+            if(i == 0) quotedArgs.append('"');
             if (quoted) quotedArgs.append('"'); else quotedArgs.append(arg);
-            quotedArgs.append(' ');
+
+            windowsCommand.add(quotedArgs, mask.get(i));
         }
         // (comment copied from old code in hudson.tasks.Ant)
         // on Windows, executing batch file can't return the correct error code,
         // so we need to wrap it into cmd.exe.
         // double %% is needed because we want ERRORLEVEL to be expanded after
         // batch file executed, not before. This alone shows how broken Windows is...
-        quotedArgs.append("&& exit %%ERRORLEVEL%%");
-        return new ArgumentListBuilder().add("cmd.exe", "/C").addQuoted(quotedArgs.toString());
+        windowsCommand.add("&&").add("exit").add("%%ERRORLEVEL%%\"");
+        return windowsCommand;
     }
 
     /**

diff --git a/core/src/test/java/hudson/util/ArgumentListBuilderTest.java b/core/src/test/java/hudson/util/ArgumentListBuilderTest.java
@@ -31,7 +31,10 @@
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+
+import org.junit.Ignore;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 
 public class ArgumentListBuilderTest {
 
@@ -101,28 +104,51 @@ public void assertPrependBeforeAddingMasked() {
 
     @Test
     public void testToWindowsCommand() {
-        ArgumentListBuilder builder = new ArgumentListBuilder(
-                "ant.bat", "-Dfoo1=abc",  // nothing special, no quotes
-                "-Dfoo2=foo bar", "-Dfoo3=/u*r", "-Dfoo4=/us?",  // add quotes
-                                                 "-Dfoo10=bar,baz",
-                "-Dfoo5=foo;bar^baz", "-Dfoo6=<xml>&here;</xml>", // add quotes
-                "-Dfoo7=foo|bar\"baz", // add quotes and "" for "
-                "-Dfoo8=% %QED% %comspec% %-%(%.%", // add quotes, and extra quotes for %Q and %c
-                "-Dfoo9=%'''%%@%"); // no quotes as none of the % are followed by a letter
+    	ArgumentListBuilder builder = new ArgumentListBuilder().
+        		add("ant.bat").add("-Dfoo1=abc").  // nothing special, no quotes
+                add("-Dfoo2=foo bar").add("-Dfoo3=/u*r").add("-Dfoo4=/us?").  // add quotes
+                add("-Dfoo10=bar,baz").
+                add("-Dfoo5=foo;bar^baz").add("-Dfoo6=<xml>&here;</xml>"). // add quotes
+                add("-Dfoo7=foo|bar\"baz"). // add quotes and "" for "
+                add("-Dfoo8=% %QED% %comspec% %-%(%.%"). // add quotes, and extra quotes for %Q and %c
+                add("-Dfoo9=%'''%%@%"); // no quotes as none of the % are followed by a letter
         // By default, does not escape %VAR%
         assertThat(builder.toWindowsCommand().toCommandArray(), is(new String[] { "cmd.exe", "/C",
-                "\"ant.bat -Dfoo1=abc \"-Dfoo2=foo bar\""
-                + " \"-Dfoo3=/u*r\" \"-Dfoo4=/us?\" \"-Dfoo10=bar,baz\" \"-Dfoo5=foo;bar^baz\""
-                + " \"-Dfoo6=<xml>&here;</xml>\" \"-Dfoo7=foo|bar\"\"baz\""
-                + " \"-Dfoo8=% %QED% %comspec% %-%(%.%\""
-                + " -Dfoo9=%'''%%@% && exit %%ERRORLEVEL%%\"" }));
+                "\"ant.bat", "-Dfoo1=abc", "\"-Dfoo2=foo bar\"", "\"-Dfoo3=/u*r\"", "\"-Dfoo4=/us?\"", 
+                "\"-Dfoo10=bar,baz\"", "\"-Dfoo5=foo;bar^baz\"", "\"-Dfoo6=<xml>&here;</xml>\"", 
+                "\"-Dfoo7=foo|bar\"\"baz\"", "\"-Dfoo8=% %QED% %comspec% %-%(%.%\"", 
+                "-Dfoo9=%'''%%@%", "&&", "exit", "%%ERRORLEVEL%%\"" }));
         // Pass flag to escape %VAR%
         assertThat(builder.toWindowsCommand(true).toCommandArray(), is(new String[] { "cmd.exe", "/C",
-                "\"ant.bat -Dfoo1=abc \"-Dfoo2=foo bar\""
-                + " \"-Dfoo3=/u*r\" \"-Dfoo4=/us?\" \"-Dfoo10=bar,baz\" \"-Dfoo5=foo;bar^baz\""
-                + " \"-Dfoo6=<xml>&here;</xml>\" \"-Dfoo7=foo|bar\"\"baz\""
-                + " \"-Dfoo8=% %\"Q\"ED% %\"c\"omspec% %-%(%.%\""
-                + " -Dfoo9=%'''%%@% && exit %%ERRORLEVEL%%\"" }));
+                "\"ant.bat", "-Dfoo1=abc", "\"-Dfoo2=foo bar\"", "\"-Dfoo3=/u*r\"", "\"-Dfoo4=/us?\"", 
+                "\"-Dfoo10=bar,baz\"", "\"-Dfoo5=foo;bar^baz\"", "\"-Dfoo6=<xml>&here;</xml>\"", 
+                "\"-Dfoo7=foo|bar\"\"baz\"", "\"-Dfoo8=% %\"Q\"ED% %\"c\"omspec% %-%(%.%\"", 
+                "-Dfoo9=%'''%%@%", "&&", "exit", "%%ERRORLEVEL%%\"" }));
+        // Try to hide password
+        builder.add("-Dpassword=hidden", true);
+        // By default, does not escape %VAR%
+        assertThat(builder.toWindowsCommand().toString(), is("cmd.exe /C \"ant.bat -Dfoo1=abc \"\"-Dfoo2=foo bar\"\" \"-Dfoo3=/u*r\" \"-Dfoo4=/us?\" \"-Dfoo10=bar,baz\" \"-Dfoo5=foo;bar^baz\" \"-Dfoo6=<xml>&here;</xml>\" \"-Dfoo7=foo|bar\"\"baz\" \"\"-Dfoo8=% %QED% %comspec% %-%(%.%\"\" -Dfoo9=%'''%%@% ****** && exit %%ERRORLEVEL%%\"" ));
+        // Pass flag to escape %VAR%
+        assertThat(builder.toWindowsCommand(true).toString(), is("cmd.exe /C \"ant.bat -Dfoo1=abc \"\"-Dfoo2=foo bar\"\" \"-Dfoo3=/u*r\" \"-Dfoo4=/us?\" \"-Dfoo10=bar,baz\" \"-Dfoo5=foo;bar^baz\" \"-Dfoo6=<xml>&here;</xml>\" \"-Dfoo7=foo|bar\"\"baz\" \"\"-Dfoo8=% %\"Q\"ED% %\"c\"omspec% %-%(%.%\"\" -Dfoo9=%'''%%@% ****** && exit %%ERRORLEVEL%%\""));
+    }
+
+    @Test
+    @Ignore
+    @Issue("JENKINS-28790")
+    public void testToWindowsCommandMasked() {
+        ArgumentListBuilder builder = new ArgumentListBuilder().
+        		add("ant.bat").add("-Dfoo1=abc").  // nothing special, no quotes
+                add("-Dfoo2=foo bar").add("-Dfoo3=/u*r").add("-Dfoo4=/us?").  // add quotes
+                add("-Dfoo10=bar,baz").
+                add("-Dfoo5=foo;bar^baz").add("-Dfoo6=<xml>&here;</xml>"). // add quotes
+                add("-Dfoo7=foo|bar\"baz"). // add quotes and "" for "
+                add("-Dfoo8=% %QED% %comspec% %-%(%.%"). // add quotes, and extra quotes for %Q and %c
+                add("-Dfoo9=%'''%%@%"). // no quotes as none of the % are followed by a letter
+                add("-Dpassword=hidden", true);
+        // By default, does not escape %VAR%
+        assertThat(builder.toWindowsCommand().toString(), is("cmd.exe /C \"ant.bat -Dfoo1=abc \"\"-Dfoo2=foo bar\"\" \"-Dfoo3=/u*r\" \"-Dfoo4=/us?\" \"-Dfoo10=bar,baz\" \"-Dfoo5=foo;bar^baz\" \"-Dfoo6=<xml>&here;</xml>\" \"-Dfoo7=foo|bar\"\"baz\" \"\"-Dfoo8=% %QED% %comspec% %-%(%.%\"\" -Dfoo9=%'''%%@% ****** && exit %%ERRORLEVEL%%\"" ));
+        // Pass flag to escape %VAR%
+        assertThat(builder.toWindowsCommand(true).toString(), is("cmd.exe /C \"ant.bat -Dfoo1=abc \"\"-Dfoo2=foo bar\"\" \"-Dfoo3=/u*r\" \"-Dfoo4=/us?\" \"-Dfoo10=bar,baz\" \"-Dfoo5=foo;bar^baz\" \"-Dfoo6=<xml>&here;</xml>\" \"-Dfoo7=foo|bar\"\"baz\" \"\"-Dfoo8=% %\"Q\"ED% %\"c\"omspec% %-%(%.%\"\" -Dfoo9=%'''%%@% ****** && exit %%ERRORLEVEL%%\""));
     }
 
     @Test