Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[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)
  • Loading branch information
escoem authored and olivergondza committed Mar 30, 2016
1 parent 951729c commit 0782aa5
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 29 deletions.
26 changes: 15 additions & 11 deletions core/src/main/java/hudson/util/ArgumentListBuilder.java
Expand Up @@ -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;
}

/**
Expand Down
62 changes: 44 additions & 18 deletions core/src/test/java/hudson/util/ArgumentListBuilderTest.java
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0782aa5

Please sign in to comment.