[FIXED JENKINS-7442] Change ArgumentListBuilder.toWindowsCommand() to not escape %VAR%

type references by default (can now be done by passing "true" parameter).
This restores ability to use %VAR% references for ant build steps on windows.

changelog.html

@@ -80,6 +80,9 @@ Upcoming changes</a>
   <li class=bug>
     Email fails when sending to multiple recipients if _any_ of them are in error
     (<a href="http://issues.jenkins-ci.org/browse/JENKINS-9006">issue 9006</a>)
+  <li class=bug>
+    Ant properties with Windows %VAR% type variables did not expand since 1.370.
+    (<a href="http://issues.jenkins-ci.org/browse/JENKINS-7442">issue 7442</a>)
   <li class=bug>
     Fixed a concurrent data access corruption in crumb generation.
   <li class=rfe>

core/src/main/java/hudson/util/ArgumentListBuilder.java

@@ -267,22 +267,25 @@ public class ArgumentListBuilder implements Serializable {
      * is needed since the command is now passed as a string to the CMD.EXE shell.
      * This is done as follows:
      * Wrap arguments in double quotes if they contain any of:
-     *   space *?,;^&<>|" or % followed by a letter.
+     *   space *?,;^&<>|"
+     *   and if escapeVars is true, % followed by a letter.
      * <br/> When testing from command prompt, these characters also need to be
      * prepended with a ^ character: ^&<>|  -- however, invoking cmd.exe from
-     * Hudson does not seem to require this extra escaping so it is not added by
+     * Jenkins does not seem to require this extra escaping so it is not added by
      * this method.
      * <br/> A " is prepended with another " character.  Note: Windows has issues
      * escaping some combinations of quotes and spaces.  Quotes should be avoided.
-     * <br/> A % followed by a letter has that letter wrapped in double quotes,
-     * to avoid possible variable expansion.  ie, %foo% becomes "%"f"oo%".
-     * The second % does not need special handling because it is not followed
-     * by a letter. <br/>
+     * <br/> If escapeVars is true, a % followed by a letter has that letter wrapped
+     * in double quotes, to avoid possible variable expansion.
+     * ie, %foo% becomes "%"f"oo%".  The second % does not need special handling
+     * because it is not followed by a letter. <br/>
      * Example: "-Dfoo=*abc?def;ghi^jkl&mno<pqr>stu|vwx""yz%"e"nd"
+     * @param escapeVars True to escape %VAR% references; false to leave these alone
+     *                   so they may be expanded when the command is run
      * @return new ArgumentListBuilder that runs given command through cmd.exe /C
      * @since 1.386
      */
-    public ArgumentListBuilder toWindowsCommand() {
+    public ArgumentListBuilder toWindowsCommand(boolean escapeVars) {
         StringBuilder quotedArgs = new StringBuilder();
         boolean quoted, percent;
         for (String arg : args) {
@@ -300,7 +303,8 @@ public class ArgumentListBuilder implements Serializable {
                     if (!quoted) quoted = startQuoting(quotedArgs, arg, i);
                     quotedArgs.append('"');
                 }
-                else if (percent && ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'))) {
+                else if (percent && escapeVars
+                         && ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z'))) {
                     if (!quoted) quoted = startQuoting(quotedArgs, arg, i);
                     quotedArgs.append('"').append(c);
                     c = '"';
@@ -320,6 +324,14 @@ public class ArgumentListBuilder implements Serializable {
         return new ArgumentListBuilder().add("cmd.exe", "/C").addQuoted(quotedArgs.toString());
     }
 
+    /**
+     * Calls toWindowsCommand(false)
+     * @see #toWindowsCommand(boolean)
+     */
+    public ArgumentListBuilder toWindowsCommand() {
+        return toWindowsCommand(false);
+    }
+
     private static boolean startQuoting(StringBuilder buf, String arg, int atIndex) {
         buf.append('"').append(arg.substring(0, atIndex));
         return true;

core/src/test/java/hudson/util/ArgumentListBuilderTest.java

@@ -119,13 +119,22 @@ public class ArgumentListBuilderTest extends Assert {
                 "-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
+        // By default, does not escape %VAR%
         assertArrayEquals(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% %-%(%.%\""
+                + " \"-Dfoo8=% %QED% %comspec% %-%(%.%\""
                 + " -Dfoo9=%'''%%@% && exit %%ERRORLEVEL%%\"" },
                 builder.toWindowsCommand().toCommandArray());
+        // Pass flag to escape %VAR%
+        assertArrayEquals(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%%\"" },
+                builder.toWindowsCommand(true).toCommandArray());
     }
 
     @Test

test/src/test/java/hudson/tasks/AntTest.java

@@ -26,6 +26,7 @@ package hudson.tasks;
 import com.gargoylesoftware.htmlunit.html.HtmlButton;
 import com.gargoylesoftware.htmlunit.html.HtmlForm;
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
+import hudson.Functions;
 import hudson.matrix.Axis;
 import hudson.matrix.AxisList;
 import hudson.matrix.MatrixRun;
@@ -43,6 +44,7 @@ import hudson.tools.InstallSourceProperty;
 import hudson.tools.ToolProperty;
 import hudson.tools.ToolPropertyDescriptor;
 import hudson.util.DescribableList;
+import org.jvnet.hudson.test.Bug;
 import org.jvnet.hudson.test.ExtractResourceSCM;
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.SingleFileSCM;
@@ -145,8 +147,9 @@ public class AntTest extends HudsonTestCase {
         assertTrue("Missing $BUILD_NUMBER: " + log, log.contains("vNUM=1"));
         assertTrue("Missing $BUILD_ID: " + log, log.contains("vID=2")); // Assuming the year starts with 2!
         assertTrue("Missing $JOB_NAME: " + log, log.contains(project.getName()));
-        // Odd build tag, but it's constructed with getParent().getName() and the parent is the matrix
-        // configuration, not the project.. if matrix build tag ever changes, update expected value here:
+        // Odd build tag, but it's constructed with getParent().getName() and the parent is the
+        // matrix configuration, not the project.. if matrix build tag ever changes, update
+        // expected value here:
         assertTrue("Missing $BUILD_TAG: " + log, log.contains("vTAG=jenkins-AX=is-1"));
         assertTrue("Missing $EXECUTOR_NUMBER: " + log, log.matches("(?s).*vEXEC=\\d.*"));
         // $NODE_NAME is expected to be empty when running on master.. not checking.
@@ -161,4 +164,43 @@ public class AntTest extends HudsonTestCase {
         assertTrue("Missing build parameter $FOO: " + log, log.contains("vFOO=bar"));
         assertTrue("Missing matrix axis $AX: " + log, log.contains("vAX=is"));
     }
+
+    public void testParameterExpansionByShell() throws Exception {
+        String antName = configureDefaultAnt().getName();
+        FreeStyleProject project = createFreeStyleProject();
+        project.setScm(new ExtractResourceSCM(getClass().getResource("ant-job.zip")));
+        String homeVar = Functions.isWindows() ? "%HOME%" : "$HOME";
+        project.addProperty(new ParametersDefinitionProperty(
+                new StringParameterDefinition("vFOO", homeVar, ""),
+                new StringParameterDefinition("vBAR", "Home sweet " + homeVar + ".", "")));
+        project.getBuildersList().add(new Ant("", antName, null, null,
+                "vHOME=" + homeVar + "\nvFOOHOME=Foo " + homeVar + "\n"));
+        FreeStyleBuild build = project.scheduleBuild2(0, new UserCause()).get();
+        assertBuildStatusSuccess(build);
+        String log = getLog(build);
+        if (!Functions.isWindows()) homeVar = "\\" + homeVar; // Regex escape for $
+        assertTrue("Missing simple HOME parameter: " + log,
+                   log.matches("(?s).*vFOO=(?!" + homeVar + ").*"));
+        assertTrue("Missing HOME parameter with other text: " + log,
+                   log.matches("(?s).*vBAR=Home sweet (?!" + homeVar + ")[^\\r\\n]*\\..*"));
+        assertTrue("Missing HOME ant property: " + log,
+                   log.matches("(?s).*vHOME=(?!" + homeVar + ").*"));
+        assertTrue("Missing HOME ant property with other text: " + log,
+                   log.matches("(?s).*vFOOHOME=Foo (?!" + homeVar + ").*"));
+    }
+
+    @Bug(7108)
+    public void testEscapeXmlInParameters() throws Exception {
+        String antName = configureDefaultAnt().getName();
+        FreeStyleProject project = createFreeStyleProject();
+        project.setScm(new ExtractResourceSCM(getClass().getResource("ant-job.zip")));
+        project.addProperty(new ParametersDefinitionProperty(
+                new StringParameterDefinition("vFOO", "<xml/>", "")));
+        project.getBuildersList().add(new Ant("", antName, null, null, "vBAR=<xml/>\n"));
+        FreeStyleBuild build = project.scheduleBuild2(0, new UserCause()).get();
+        assertBuildStatusSuccess(build);
+        String log = getLog(build);
+        assertTrue("Missing parameter: " + log, log.contains("vFOO=<xml/>"));
+        assertTrue("Missing ant property: " + log, log.contains("vBAR=<xml/>"));
+    }
 }

war/src/main/webapp/help/ant/ant-properties.html

@@ -7,8 +7,9 @@ name1=value1
 name2=$VAR2
 </pre>
   These are passed to Ant like <tt>"-Dname1=value1 -Dname2=value2"</tt>.
-  Always use <tt>$VAR</tt> style for parameter references (do not use %VAR% style,
-  even when build runs on Windows).
+  Always use <tt>$VAR</tt> style (even on Windows) for references to Jenkins-defined
+  environment variables.  On Windows, <tt>%VAR%</tt> style references may be used
+  for environment variables that exist outside of Jenkins.
   Backslashes are used for escaping, so use <tt>\\</tt> for a single backslash.
   Double quotes (") should be avoided, as ant on *nix wraps parameters in quotes
   quotes and runs them through <tt>eval</tt>, and Windows has its own issues