gnodet-bot commented on code in PR #13652:
URL: https://github.com/apache/maven/pull/13652#discussion_r4334419339
##########
apache-maven/src/assembly/maven/bin/JvmConfigParser.java:
##########
@@ -128,14 +129,23 @@ private static String processLine(String line, String
mavenProjectBasedir) {
}
/**
- * Append parsed arguments as quoted strings to the result builder.
+ * Append parsed arguments in Java @argfile format: one argument per line.
+ * Arguments containing spaces or double-quotes are quoted; pipes and other
+ * special characters are literal (not interpreted by the shell).
*/
private static void appendQuotedArguments(StringBuilder result,
List<String> args) {
for (String arg : args) {
if (result.length() > 0) {
- result.append(' ');
+ result.append(System.lineSeparator());
+ }
+ // Quote the argument if it contains whitespace or double-quotes
+ boolean needsQuoting = arg.indexOf(' ') >= 0 || arg.indexOf('\t')
>= 0 || arg.indexOf('"') >= 0;
Review Comment:
⚠️ **Unix regression — shell metacharacters now unquoted in `MAVEN_OPTS`**
The old code always double-quoted every arg. The new `needsQuoting` only
checks for space, tab, and double-quote. Args like
`-Dhttp.nonProxyHosts=de|*.de` (pipe, no space/quote) are output **unquoted**.
On Unix, `mvn` captures this stdout, assigns it unquoted to `MAVEN_OPTS`, then
does `eval exec "$cmd"`. Eval re-parses the command as shell code → unquoted
`|` = shell pipe → Maven fails on Unix.
Fix: extend the check to include shell metacharacters. Quoting is always
valid in `@argfile` format, so this is safe for both consumers:
```suggestion
// Quote the argument if it contains whitespace, double-quotes,
or any shell
// metacharacter that eval would re-interpret on Unix (|, &, ;,
<, >, `, $).
// Quoting is always valid in @argfile format, so this is safe
for both the
// Windows @argfile consumer and the Unix eval-based consumer in
bin/mvn.
boolean needsQuoting = arg.indexOf(' ') >= 0 ||
arg.indexOf('\t') >= 0
|| arg.indexOf('"') >= 0 || arg.indexOf('|') >= 0
|| arg.indexOf('&') >= 0 || arg.indexOf(';') >= 0
|| arg.indexOf('<') >= 0 || arg.indexOf('>') >= 0
|| arg.indexOf('`') >= 0 || arg.indexOf('$') >= 0
|| arg.endsWith("\\\\");
```
##########
apache-maven/src/assembly/maven/bin/JvmConfigParser.java:
##########
@@ -128,14 +129,23 @@ private static String processLine(String line, String
mavenProjectBasedir) {
}
/**
- * Append parsed arguments as quoted strings to the result builder.
+ * Append parsed arguments in Java @argfile format: one argument per line.
+ * Arguments containing spaces or double-quotes are quoted; pipes and other
+ * special characters are literal (not interpreted by the shell).
*/
private static void appendQuotedArguments(StringBuilder result,
List<String> args) {
for (String arg : args) {
if (result.length() > 0) {
- result.append(' ');
+ result.append(System.lineSeparator());
+ }
+ // Quote the argument if it contains whitespace or double-quotes
+ boolean needsQuoting = arg.indexOf(' ') >= 0 || arg.indexOf('\t')
>= 0 || arg.indexOf('"') >= 0;
+ if (needsQuoting) {
+ // Escape embedded double-quotes as \" per @argfile spec
+ result.append('"').append(arg.replace("\"",
"\\\"")).append('"');
Review Comment:
⚠️ **`@argfile` trailing-backslash not escaped — line-continuation bug**
Per the Java `@argfile` spec: backslash must be escaped as `\\\\`, and `\\`
at end-of-line is a line-continuation. This code does not escape backslashes.
For an arg ending in `\\` (e.g. `-Dpath=C:\\Users\\foo\\`):
- Unquoted output: `arg\\` + newline → line continuation → next arg
concatenated to this one.
- Quoted output: `"arg\\"` → the trailing `\\"` is an escaped quote → string
never closes → parse error.
Fix: escape backslashes before double-quotes in the quoting branch:
```suggestion
if (needsQuoting) {
// Escape backslashes first (in @argfile, \\\\ is a literal
backslash, and a
// trailing \\ before the closing quote would be misread as
an escaped quote).
// Then escape embedded double-quotes as \\" per @argfile
spec.
String escaped = arg.replace("\\\\",
"\\\\\\\\").replace("\"", "\\\\\"");
result.append('"').append(escaped).append('"');
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
