gnodet-bot commented on code in PR #13652:
URL: https://github.com/apache/maven/pull/13652#discussion_r4347733646
##########
apache-maven/src/assembly/maven/bin/JvmConfigParser.java:
##########
@@ -128,7 +128,10 @@ private static String processLine(String line, String
mavenProjectBasedir) {
}
/**
- * Append parsed arguments as quoted strings to the result builder.
+ * Append parsed arguments in Java {@code @argfile} format: one argument
per line.
+ * Arguments are quoted when they contain whitespace, double-quotes, or
backslashes.
+ * Quoting is always valid in {@code @argfile} format and ensures safe
parsing
+ * by both the Windows and Unix JVM {@code @argfile} consumers.
Review Comment:
**Javadoc/implementation mismatch:** The Javadoc says *"Arguments are quoted
when they contain whitespace, double-quotes, or backslashes"*, implying
selective/conditional quoting. But the implementation quotes every argument
unconditionally — which is fine. Fix the Javadoc to match the actual behavior,
and document the backslash escaping requirement (see the inline comment below
on the method body):
```suggestion
* Append parsed arguments in Java {@code @argfile} format.
* Every argument is wrapped in double-quotes. Per the JDK {@code
@argfile} spec,
* backslashes inside double-quoted strings are escape characters and
must be
* doubled ({@code \\}) to be treated as literal backslashes.
```
##########
apache-maven/src/assembly/maven/bin/JvmConfigParser.java:
##########
@@ -128,7 +128,10 @@ private static String processLine(String line, String
mavenProjectBasedir) {
}
/**
- * Append parsed arguments as quoted strings to the result builder.
+ * Append parsed arguments in Java {@code @argfile} format: one argument
per line.
+ * Arguments are quoted when they contain whitespace, double-quotes, or
backslashes.
+ * Quoting is always valid in {@code @argfile} format and ensures safe
parsing
+ * by both the Windows and Unix JVM {@code @argfile} consumers.
*/
private static void appendQuotedArguments(StringBuilder result,
List<String> args) {
Review Comment:
🐛 **`appendQuotedArguments` does not escape backslashes — corrupts Windows
paths in @argfile.**
In Java `@argfile` format (JDK 9+), inside a **double-quoted string**,
backslash is an **escape character**. The JDK launcher source (`args.c`)
processes: `\f` → form-feed, `\n` → newline, `\r` → CR, `\t` → tab, `\\` →
literal backslash; unrecognised escapes like `\U`, `\a` silently corrupt the
character. Reference: Jonathan Gibbons (JDK launcher engineer), JDK-8216522:
*"inside quotes, the backslash character is an escape character, and must be
doubled to pass through a backslash character."*
The `processLine` method injects `mavenProjectBasedir` (a Windows path like
`C:\Users\alice\project`) via `${MAVEN_PROJECTBASEDIR}` substitution. That path
reaches `appendQuotedArguments`, which wraps it in quotes without escaping the
backslashes. The argfile then contains `"-Dmy.prop=C:\Users\alice\project"` →
the JVM receives `-Dmy.prop=C:` + (corrupted characters).
Fix — escape backslashes before wrapping in double-quotes:
```suggestion
private static void appendQuotedArguments(StringBuilder result,
List<String> args) {
for (String arg : args) {
if (result.length() > 0) {
result.append(' ');
}
result.append('"').append(arg.replace("\", "\\")).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]
