Skip to content

Commit 937fcf2

Browse files
gnodetgnodet
committed
[MNG-11363] Fix pipe symbols and special characters in .mvn/jvm.config
This commit unifies the .mvn/jvm.config parsing approach by using JvmConfigParser.java on both Unix and Windows platforms, solving multiple related issues with special character handling. Issues Fixed: - MNG-11363: Pipe symbols (|) in jvm.config cause shell parsing errors - MNG-11485: Non-POSIX xargs -0 fails on AIX, FreeBSD, and other systems - MNG-11486: sed command fails when paths contain @ symbols Changes: - Modified mvn (Unix): Replaced awk/sed pipeline with JvmConfigParser.java compilation and execution. This ensures POSIX compliance and handles all special characters (pipes, quotes, @, etc.) correctly. - Modified mvn.cmd (Windows): Removed debug output and updated comments to reflect all special characters now handled correctly. - Added integration test MavenITgh11485AtSignInJvmConfigTest to verify @ symbol handling in jvm.config files (important for Jenkins workspaces like workspace/project_PR-350@2). The Java-based parser approach: - Handles comment removal (both # at start and end-of-line) - Performs variable substitution (${MAVEN_PROJECTBASEDIR}) - Correctly parses quoted arguments with special characters - Works consistently across all platforms - Is POSIX-compliant (no xargs -0, awk, or complex sed needed) Performance Impact: Unix script is ~200ms slower (320ms → 520ms for mvn -v) due to Java compilation, but this is acceptable for the correctness, maintainability, and cross-platform consistency benefits. Related PRs: - Fixes apache#11365 (MNG-11363: pipe symbols) - Fixes apache#11489 (MNG-11485: POSIX compliance) - Fixes apache#11499 (MNG-11486: @ symbols in paths)
1 parent 178c6fa commit 937fcf2

File tree

5 files changed

+195
-39
lines changed

5 files changed

+195
-39
lines changed

‎apache-maven/src/assembly/maven/bin/mvn‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -166,37 +166,42 @@ find_file_argument_basedir() {
166166
}
167167

168168
# concatenates all lines of a file and replaces variables
169+
# Uses Java-based parser to handle all special characters correctly
170+
# This avoids shell parsing issues with pipes, quotes, @, and other special characters
171+
# and ensures POSIX compliance (no xargs -0, awk, or complex sed needed)
169172
concat_lines() {
170173
if [ -f "$1" ]; then
171-
# Convert CR to LF, remove empty lines and comments, perform variable substitution,
172-
# and escape pipe symbols for eval
173-
tr '\r' '\n' < "$1" | \
174-
sed -e '/^$/d' -e 's/#.*$//' | \
175-
sed \
176-
-e "s@\${MAVEN_PROJECTBASEDIR}@$MAVEN_PROJECTBASEDIR@g" \
177-
-e "s@\$MAVEN_PROJECTBASEDIR@$MAVEN_PROJECTBASEDIR@g" | \
178-
awk '
179-
{
180-
result = ""
181-
in_quotes = 0
182-
for (i = 1; i <= length($0); i++) {
183-
char = substr($0, i, 1)
184-
if (char == "\"") {
185-
in_quotes = !in_quotes
186-
result = result char
187-
} else if (char == "|" && !in_quotes) {
188-
# Escape unquoted pipes for eval
189-
result = result "\\|"
190-
} else {
191-
result = result char
192-
}
193-
}
194-
# Accumulate lines with space separator
195-
if (NR > 1) printf " "
196-
printf "%s", result
174+
# Compile and run JvmConfigParser
175+
# Use a temporary directory for compilation to avoid conflicts
176+
jvm_parser_dir="${TMPDIR:-/tmp}/mvn-jvm-parser-$$"
177+
mkdir -p "$jvm_parser_dir"
178+
179+
# Compile the parser
180+
"$JAVACMD" -version >/dev/null 2>&1 || {
181+
echo "Error: Java not found. Please set JAVA_HOME." >&2
182+
return 1
197183
}
198-
END { if (NR > 0) print "" }
199-
'
184+
185+
javac_cmd="${JAVACMD%java*}javac"
186+
if [ ! -x "$javac_cmd" ]; then
187+
# Try to find javac in JAVA_HOME
188+
if [ -n "$JAVA_HOME" ]; then
189+
javac_cmd="$JAVA_HOME/bin/javac"
190+
fi
191+
fi
192+
193+
"$javac_cmd" -d "$jvm_parser_dir" "$MAVEN_HOME/bin/JvmConfigParser.java" >/dev/null 2>&1
194+
if [ $? -eq 0 ]; then
195+
# Run the parser and capture output
196+
result=$("$JAVACMD" -cp "$jvm_parser_dir" JvmConfigParser "$1" "$MAVEN_PROJECTBASEDIR" 2>/dev/null)
197+
# Clean up
198+
rm -rf "$jvm_parser_dir"
199+
echo "$result"
200+
else
201+
# Fallback: if compilation fails, just skip jvm.config
202+
rm -rf "$jvm_parser_dir"
203+
echo "Warning: Failed to compile JvmConfigParser, skipping jvm.config" >&2
204+
fi
200205
fi
201206
}
202207

‎apache-maven/src/assembly/maven/bin/mvn.cmd‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,26 +183,18 @@ set JVM_CONFIG_MAVEN_OPTS=
183183
if not exist "%MAVEN_PROJECTBASEDIR%\.mvn\jvm.config" goto endReadJvmConfig
184184

185185
rem Use Java to parse jvm.config to avoid batch script parsing issues with special characters
186-
rem This handles pipes, quotes, and other special characters correctly
187-
rem Use random temp directory to avoid conflicts between different Maven versions
186+
rem This handles pipes, quotes, @, and other special characters correctly
187+
rem Use random temp directory to avoid conflicts between different Maven invocations
188188
set "JVM_CONFIG_PARSER_DIR=%TEMP%\mvn-jvm-parser-%RANDOM%-%RANDOM%"
189189
mkdir "%JVM_CONFIG_PARSER_DIR%"
190190
set "JVM_CONFIG_TEMP=%TEMP%\mvn-jvm-config-%RANDOM%.txt"
191-
set "JVM_CONFIG_ERR=%TEMP%\mvn-jvm-config-err-%RANDOM%.txt"
192191
"%JAVACMD:java.exe=javac.exe%" -d "%JVM_CONFIG_PARSER_DIR%" "%MAVEN_HOME%\bin\JvmConfigParser.java" >nul 2>&1
193-
"%JAVACMD%" -cp "%JVM_CONFIG_PARSER_DIR%" JvmConfigParser "%MAVEN_PROJECTBASEDIR%\.mvn\jvm.config" "%MAVEN_PROJECTBASEDIR%" > "%JVM_CONFIG_TEMP%" 2> "%JVM_CONFIG_ERR%"
192+
"%JAVACMD%" -cp "%JVM_CONFIG_PARSER_DIR%" JvmConfigParser "%MAVEN_PROJECTBASEDIR%\.mvn\jvm.config" "%MAVEN_PROJECTBASEDIR%" > "%JVM_CONFIG_TEMP%" 2>nul
194193
rem Read the single line from temp file
195194
set /p JVM_CONFIG_MAVEN_OPTS=<"%JVM_CONFIG_TEMP%"
196195

197-
rem Debug output to file for IT verification
198-
echo JVM_CONFIG_MAVEN_OPTS=%JVM_CONFIG_MAVEN_OPTS% > "%MAVEN_PROJECTBASEDIR%\mvn-debug.txt"
199-
echo MAVEN_OPTS=%MAVEN_OPTS% >> "%MAVEN_PROJECTBASEDIR%\mvn-debug.txt"
200-
echo PARSER_STDERR: >> "%MAVEN_PROJECTBASEDIR%\mvn-debug.txt"
201-
type "%JVM_CONFIG_ERR%" >> "%MAVEN_PROJECTBASEDIR%\mvn-debug.txt" 2>nul
202-
203196
rem Cleanup temp files and directory
204197
del "%JVM_CONFIG_TEMP%" 2>nul
205-
del "%JVM_CONFIG_ERR%" 2>nul
206198
rmdir /s /q "%JVM_CONFIG_PARSER_DIR%" 2>nul
207199

208200
:endReadJvmConfig

‎its/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh11485AtSignInJvmConfigTest.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.it;
20+
21+
import java.io.File;
22+
import java.util.Properties;
23+
24+
import org.junit.jupiter.api.Test;
25+
26+
import static org.junit.jupiter.api.Assertions.assertEquals;
27+
28+
/**
29+
* This is a test set for <a href="https://github.com/apache/maven/issues/11485">GH-11485</a>:
30+
* Verify that @ character in .mvn/jvm.config values is handled correctly.
31+
* This is important for Jenkins workspaces like workspace/project_PR-350@2
32+
*/
33+
public class MavenITgh11485AtSignInJvmConfigTest extends AbstractMavenIntegrationTestCase {
34+
35+
@Test
36+
public void testAtSignInJvmConfig() throws Exception {
37+
File testDir = extractResources("/gh-11485-at-sign");
38+
39+
Verifier verifier = newVerifier(testDir.getAbsolutePath());
40+
verifier.addCliArgument(
41+
"-Dexpression.outputFile=" + new File(testDir, "target/pom.properties").getAbsolutePath());
42+
verifier.setForkJvm(true); // custom .mvn/jvm.config
43+
verifier.addCliArgument("validate");
44+
verifier.execute();
45+
verifier.verifyErrorFreeLog();
46+
47+
Properties props = verifier.loadProperties("target/pom.properties");
48+
String expectedPath = testDir.getAbsolutePath().replace('\\', '/');
49+
assertEquals(
50+
expectedPath + "/workspace@2/test",
51+
props.getProperty("project.properties.pathWithAtProp").replace('\\', '/'),
52+
"Path with @ character should be preserved");
53+
assertEquals(
54+
"value@test",
55+
props.getProperty("project.properties.propWithAtProp"),
56+
"Property value with @ character should be preserved");
57+
}
58+
59+
@Test
60+
public void testAtSignInCommandLineProperty() throws Exception {
61+
File testDir = extractResources("/gh-11485-at-sign");
62+
63+
Verifier verifier = newVerifier(testDir.getAbsolutePath());
64+
verifier.addCliArgument(
65+
"-Dexpression.outputFile=" + new File(testDir, "target/pom.properties").getAbsolutePath());
66+
verifier.setForkJvm(true); // custom .mvn/jvm.config
67+
// Pass a path with @ character via command line (simulating Jenkins workspace)
68+
String jenkinsPath = testDir.getAbsolutePath().replace('\\', '/') + "/jenkins.workspace/proj@2";
69+
verifier.addCliArgument("-Dcmdline.path=" + jenkinsPath);
70+
verifier.addCliArgument("-Dcmdline.value=test@value");
71+
verifier.addCliArgument("validate");
72+
verifier.execute();
73+
verifier.verifyErrorFreeLog();
74+
75+
Properties props = verifier.loadProperties("target/pom.properties");
76+
assertEquals(
77+
jenkinsPath,
78+
props.getProperty("project.properties.cmdlinePath").replace('\\', '/'),
79+
"Command-line path with @ character should be preserved");
80+
assertEquals(
81+
"test@value",
82+
props.getProperty("project.properties.cmdlineValue"),
83+
"Command-line value with @ character should be preserved");
84+
}
85+
}
86+

‎its/core-it-suite/src/test/resources/gh-11485-at-sign/.mvn/jvm.config‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-Dpath.with.at=${MAVEN_PROJECTBASEDIR}/workspace@2/test
2+
-Dprop.with.at=value@test
3+

‎its/core-it-suite/src/test/resources/gh-11485-at-sign/pom.xml‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
Licensed to the Apache Software Foundation (ASF) under one
4+
or more contributor license agreements. See the NOTICE file
5+
distributed with this work for additional information
6+
regarding copyright ownership. The ASF licenses this file
7+
to you under the Apache License, Version 2.0 (the
8+
"License"); you may not use this file except in compliance
9+
with the License. You may obtain a copy of the License at
10+
11+
http://www.apache.org/licenses/LICENSE-2.0
12+
13+
Unless required by applicable law or agreed to in writing,
14+
software distributed under the License is distributed on an
15+
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
16+
KIND, either express or implied. See the License for the
17+
specific language governing permissions and limitations
18+
under the License.
19+
-->
20+
<project xmlns="http://maven.apache.org/POM/4.0.0"
21+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
22+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
23+
<modelVersion>4.0.0</modelVersion>
24+
25+
<groupId>org.apache.maven.its.gh11485</groupId>
26+
<artifactId>test</artifactId>
27+
<version>1.0</version>
28+
<packaging>pom</packaging>
29+
30+
<name>Test @ character in jvm.config</name>
31+
<description>
32+
Verify that @ character in jvm.config values is handled correctly.
33+
This is important for Jenkins workspaces like workspace/project_PR-350@2
34+
</description>
35+
36+
<properties>
37+
<pathWithAtProp>${path.with.at}</pathWithAtProp>
38+
<propWithAtProp>${prop.with.at}</propWithAtProp>
39+
<cmdlinePath>${cmdline.path}</cmdlinePath>
40+
<cmdlineValue>${cmdline.value}</cmdlineValue>
41+
</properties>
42+
43+
<build>
44+
<plugins>
45+
<plugin>
46+
<groupId>org.apache.maven.its.plugins</groupId>
47+
<artifactId>maven-it-plugin-expression</artifactId>
48+
<version>2.1-SNAPSHOT</version>
49+
<executions>
50+
<execution>
51+
<phase>validate</phase>
52+
<goals>
53+
<goal>eval</goal>
54+
</goals>
55+
<configuration>
56+
<outputFile>target/pom.properties</outputFile>
57+
<expressions>
58+
<expression>project/properties/pathWithAtProp</expression>
59+
<expression>project/properties/propWithAtProp</expression>
60+
<expression>project/properties/cmdlinePath</expression>
61+
<expression>project/properties/cmdlineValue</expression>
62+
</expressions>
63+
</configuration>
64+
</execution>
65+
</executions>
66+
</plugin>
67+
</plugins>
68+
</build>
69+
</project>
70+

0 commit comments

Comments
 (0)