Skip to content

Commit 0d4dc41

Browse files
committed
- escaped comman and semicolon in CommandBuilderUtils.java
- added random string to the temporary filename - double-quotation followed by `cmd /c` did not worked properly - no need to escape `=` by `^` - if double-quoted string ended with `\` like classpath, the last `\` is parsed as the escape charactor and the closing `"` didn't work properly
1 parent 2a332e5 commit 0d4dc41

File tree

4 files changed

+9
-11
lines changed

4 files changed

+9
-11
lines changed

bin/spark-class2.cmd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ if not "x%JAVA_HOME%"=="x" set RUNNER=%JAVA_HOME%\bin\java
5353

5454
rem The launcher library prints the command to be executed in a single line suitable for being
5555
rem executed by the batch interpreter. So read all the output of the launcher into a variable.
56-
set LAUNCHER_OUTPUT=%temp%\spark-class-launcher-output.txt
56+
set LAUNCHER_OUTPUT=%temp%\spark-class-launcher-output-%RANDOM%.txt
5757
"%RUNNER%" -cp %SPARK_ASSEMBLY_JAR% org.apache.spark.launcher.Main %* > %LAUNCHER_OUTPUT%
5858
for /f "tokens=*" %%i in (%LAUNCHER_OUTPUT%) do (
5959
set SPARK_CMD=%%i

launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ static String quoteForBatchScript(String arg) {
243243
boolean needsQuotes = false;
244244
for (int i = 0; i < arg.length(); i++) {
245245
int c = arg.codePointAt(i);
246-
if (Character.isWhitespace(c) || c == '"' || c == '=') {
246+
if (Character.isWhitespace(c) || c == '"' || c == '=' || c == ',' || c == ';') {
247247
needsQuotes = true;
248248
break;
249249
}
@@ -260,15 +260,14 @@ static String quoteForBatchScript(String arg) {
260260
quoted.append('"');
261261
break;
262262

263-
case '=':
264-
quoted.append('^');
265-
break;
266-
267263
default:
268264
break;
269265
}
270266
quoted.appendCodePoint(cp);
271267
}
268+
if (arg.codePointAt(arg.length() - 1) == '\\') {
269+
quoted.append("\\");
270+
}
272271
quoted.append("\"");
273272
return quoted.toString();
274273
}

launcher/src/main/java/org/apache/spark/launcher/Main.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,9 @@ public static void main(String[] argsArray) throws Exception {
101101
* The method quotes all arguments so that spaces are handled as expected. Quotes within arguments
102102
* are "double quoted" (which is batch for escaping a quote). This page has more details about
103103
* quoting and other batch script fun stuff: http://ss64.com/nt/syntax-esc.html
104-
*
105-
* The command is executed using "cmd /c" and formatted in single line, since that's the
106-
* easiest way to consume this from a batch script (see spark-class2.cmd).
107104
*/
108105
private static String prepareWindowsCommand(List<String> cmd, Map<String, String> childEnv) {
109-
StringBuilder cmdline = new StringBuilder("cmd /c \"");
106+
StringBuilder cmdline = new StringBuilder("");
110107
for (Map.Entry<String, String> e : childEnv.entrySet()) {
111108
cmdline.append(String.format("set %s=%s", e.getKey(), e.getValue()));
112109
cmdline.append(" && ");
@@ -115,7 +112,6 @@ private static String prepareWindowsCommand(List<String> cmd, Map<String, String
115112
cmdline.append(quoteForBatchScript(arg));
116113
cmdline.append(" ");
117114
}
118-
cmdline.append("\"");
119115
return cmdline.toString();
120116
}
121117

launcher/src/test/java/org/apache/spark/launcher/CommandBuilderUtilsSuite.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ public void testWindowsBatchQuoting() {
7575
assertEquals("\"a \"\"b\"\" c\"", quoteForBatchScript("a \"b\" c"));
7676
assertEquals("\"a\"\"b\"\"c\"", quoteForBatchScript("a\"b\"c"));
7777
assertEquals("\"ab^=\"\"cd\"\"\"", quoteForBatchScript("ab=\"cd\""));
78+
assertEquals("\"a,b,c\"", quoteForBatchScript("a,b,c"));
79+
assertEquals("\"a;b;c\"", quoteForBatchScript("a;b;c"));
80+
assertEquals("\"a,b,c\\\\\"", quoteForBatchScript("a,b,c\\"));
7881
}
7982

8083
@Test

0 commit comments

Comments
 (0)