[MNG-4559] Fix .mvn/jvm.config parsing with spaces, quotes and comments (#2213)

The .mvn/jvm.config file parsing in mvn.cmd needed improvements to properly handle multiple JVM arguments, especially when dealing with comments, quotes and spaces in values.

This PR:
- Ensures proper parsing of multiple JVM arguments from .mvn/jvm.config
- Maintains proper spacing between arguments
- Handles both inline and full-line comments correctly
- Fixes a bug in the space trimming logic (%%x -> %%i)
- Preserves spaces within quoted values

The changes are validated by MavenITmng4559MultipleJvmArgsTest which verifies:
- Multiple JVM arguments are properly handled
- Comments are correctly processed
- Arguments with spaces in quotes are preserved
- Arguments from multiple lines are properly combined

Fixes: https://issues.apache.org/jira/browse/MNG-4559

Author: gnodet

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

@@ -168,11 +168,25 @@ find_file_argument_basedir() {
 # concatenates all lines of a file and replaces variables
 concat_lines() {
   if [ -f "$1" ]; then
-    # First transform line endings to spaces
-    content=$(tr -s '\r\n' ' ' < "$1")
-    # Handle both ${var} and $var formats, only substitute MAVEN_PROJECTBASEDIR
-    echo "$content" | sed -e "s|\${MAVEN_PROJECTBASEDIR}|$MAVEN_PROJECTBASEDIR|g" \
-                         -e "s|\$MAVEN_PROJECTBASEDIR|$MAVEN_PROJECTBASEDIR|g"
+    # First convert all CR to LF using tr
+    tr '\r' '\n' < "$1" | \
+    sed -e '/^$/d' -e 's/#.*$//' | \
+    # Split into words and process each argument
+    xargs -n 1 | \
+    while read -r arg; do
+      # Replace variables first
+      arg=$(echo "$arg" | sed \
+        -e "s@\${MAVEN_PROJECTBASEDIR}@$MAVEN_PROJECTBASEDIR@g" \
+        -e "s@\$MAVEN_PROJECTBASEDIR@$MAVEN_PROJECTBASEDIR@g")
+
+      # Add quotes only if argument contains spaces and isn't already quoted
+      if echo "$arg" | grep -q " " && ! echo "$arg" | grep -q "^\".*\"$"; then
+        echo "\"$arg\""
+      else
+        echo "$arg"
+      fi
+    done | \
+    tr '\n' ' '
   fi
 }
 
@@ -224,16 +238,25 @@ handle_args() {
 handle_args "$@"
 MAVEN_MAIN_CLASS=${MAVEN_MAIN_CLASS:=org.apache.maven.cling.MavenCling}
 
-exec "$JAVACMD" \
+cmd="\"$JAVACMD\" \
   $MAVEN_OPTS \
   $MAVEN_DEBUG_OPTS \
   --enable-native-access=ALL-UNNAMED \
-  -classpath "$LAUNCHER_JAR" \
-  "-Dclassworlds.conf=$CLASSWORLDS_CONF" \
-  "-Dmaven.home=$MAVEN_HOME" \
-  "-Dmaven.mainClass=$MAVEN_MAIN_CLASS" \
-  "-Dlibrary.jline.path=${MAVEN_HOME}/lib/jline-native" \
-  "-Dmaven.multiModuleProjectDirectory=$MAVEN_PROJECTBASEDIR" \
+  -classpath \"$LAUNCHER_JAR\" \
+  \"-Dclassworlds.conf=$CLASSWORLDS_CONF\" \
+  \"-Dmaven.home=$MAVEN_HOME\" \
+  \"-Dmaven.mainClass=$MAVEN_MAIN_CLASS\" \
+  \"-Dlibrary.jline.path=${MAVEN_HOME}/lib/jline-native\" \
+  \"-Dmaven.multiModuleProjectDirectory=$MAVEN_PROJECTBASEDIR\" \
   $LAUNCHER_CLASS \
-  $MAVEN_ARGS \
-  "$@"
+  $MAVEN_ARGS"
+# Add remaining arguments with proper quoting
+for arg in "$@"; do
+    cmd="$cmd \"$arg\""
+done
+
+# Debug: print the command that will be executed
+#echo "About to execute:"
+#echo "$cmd"
+
+eval exec "$cmd"

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

@@ -177,13 +177,34 @@ if not exist "%MAVEN_PROJECTBASEDIR%\.mvn\jvm.config" goto endReadJvmConfig
 
 @setlocal EnableExtensions EnableDelayedExpansion
 set JVM_CONFIG_MAVEN_OPTS=
-for /F "usebackq delims=" %%a in ("%MAVEN_PROJECTBASEDIR%\.mvn\jvm.config") do (
+for /F "usebackq tokens=* delims=" %%a in ("%MAVEN_PROJECTBASEDIR%\.mvn\jvm.config") do (
     set "line=%%a"
-    set "line=!line:$MAVEN_PROJECTBASEDIR=%MAVEN_PROJECTBASEDIR%!"
-    set "line=!line:${MAVEN_PROJECTBASEDIR}=%MAVEN_PROJECTBASEDIR%!"
-    set JVM_CONFIG_MAVEN_OPTS=!JVM_CONFIG_MAVEN_OPTS! !line!
+    
+    rem Skip empty lines and full-line comments
+    echo !line! | findstr /b /r /c:"[ ]*#" >nul
+    if errorlevel 1 (
+        rem Handle end-of-line comments by taking everything before #
+        for /f "tokens=1* delims=#" %%i in ("!line!") do set "line=%%i"
+        
+        rem Trim leading/trailing spaces while preserving spaces in quotes
+        set "trimmed=!line!"
+        for /f "tokens=* delims= " %%i in ("!trimmed!") do set "trimmed=%%i"
+        for /l %%i in (1,1,100) do if "!trimmed:~-1!"==" " set "trimmed=!trimmed:~0,-1!"
+        
+        rem Replace MAVEN_PROJECTBASEDIR placeholders
+        set "trimmed=!trimmed:${MAVEN_PROJECTBASEDIR}=%MAVEN_PROJECTBASEDIR%!"
+        set "trimmed=!trimmed:$MAVEN_PROJECTBASEDIR=%MAVEN_PROJECTBASEDIR%!"
+        
+        if not "!trimmed!"=="" (
+            if "!JVM_CONFIG_MAVEN_OPTS!"=="" (
+                set "JVM_CONFIG_MAVEN_OPTS=!trimmed!"
+            ) else (
+                set "JVM_CONFIG_MAVEN_OPTS=!JVM_CONFIG_MAVEN_OPTS! !trimmed!"
+            )
+        )
+    )
 )
-@endlocal & set MAVEN_OPTS=%MAVEN_OPTS% %JVM_CONFIG_MAVEN_OPTS%
+@endlocal & set "MAVEN_OPTS=%MAVEN_OPTS% %JVM_CONFIG_MAVEN_OPTS%"
 
 :endReadJvmConfig
 

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

@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.it;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.util.Properties;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * This is a test set for MNG-4559:
+ * - Verifies that multiple JVM arguments in .mvn/jvm.config are properly handled
+ * - Ensures arguments are correctly split and passed to the JVM
+ */
+public class MavenITmng4559MultipleJvmArgsTest extends AbstractMavenIntegrationTestCase {
+    public MavenITmng4559MultipleJvmArgsTest() {
+        super("[4.0.0-rc-4,)");
+    }
+
+    @Test
+    void testMultipleJvmArgs() throws Exception {
+        File testDir = extractResources("/mng-4559-multiple-jvm-args");
+        File mvnDir = new File(testDir, ".mvn");
+        File jvmConfig = new File(mvnDir, "jvm.config");
+
+        mvnDir.mkdirs();
+        Files.writeString(
+                jvmConfig.toPath(),
+                "# This is a comment\n" + "-Xmx2048m -Xms1024m -Dtest.prop1=value1 # end of line comment\n"
+                        + "# Another comment\n"
+                        + "-Dtest.prop2=\"value 2\"");
+
+        Verifier verifier = newVerifier(testDir.getAbsolutePath());
+        verifier.setForkJvm(true);
+        verifier.addCliArgument("validate");
+        verifier.execute();
+        verifier.verifyErrorFreeLog();
+
+        Properties props = verifier.loadProperties("target/jvm.properties");
+        assertEquals("value1", props.getProperty("project.properties.pom.test.prop1"));
+        assertEquals("value 2", props.getProperty("project.properties.pom.test.prop2"));
+    }
+}

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

@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.it;
+
+import java.nio.file.Path;
+import java.util.Properties;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * This is a test set for <a href="https://issues.apache.org/jira/browse/MNG-4559">MNG-4559</a>.
+ */
+class MavenITmng4559SpacesInJvmOptsTest extends AbstractMavenIntegrationTestCase {
+
+    MavenITmng4559SpacesInJvmOptsTest() {
+        super("[4.0.0-rc-4,)");
+    }
+
+    /**
+     *  Verify the dependency management of the consumer POM is computed correctly
+     */
+    @Test
+    void testIt() throws Exception {
+        Path basedir =
+                extractResources("/mng-4559-spaces-jvm-opts").getAbsoluteFile().toPath();
+
+        Verifier verifier = newVerifier(basedir.toString());
+        verifier.setEnvironmentVariable("MAVEN_OPTS", "-Dprop.maven-opts=\"foo bar\"");
+        verifier.addCliArguments("validate");
+        verifier.execute();
+        verifier.verifyErrorFreeLog();
+
+        Properties props = verifier.loadProperties("target/pom.properties");
+        assertEquals("foo bar", props.getProperty("project.properties.pom.prop.jvm-opts"));
+        assertEquals("foo bar", props.getProperty("project.properties.pom.prop.maven-opts"));
+    }
+}

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

@@ -22,6 +22,7 @@
 import java.nio.file.Files;
 import java.util.Properties;
 
+import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -31,8 +32,8 @@
  * Check that the <code>.mvn/jvm.config</code> file contents are concatenated properly, no matter
  * what line endings are used.
  */
-public class MavenITmng6255FixConcatLines extends AbstractMavenIntegrationTestCase {
-    public MavenITmng6255FixConcatLines() {
+class MavenITmng6255FixConcatLines extends AbstractMavenIntegrationTestCase {
+    MavenITmng6255FixConcatLines() {
         super("[3.5.3,)");
     }
 
@@ -43,7 +44,9 @@ public MavenITmng6255FixConcatLines() {
      *
      * @throws Exception in case of failure
      */
-    public void disabledJvmConfigFileCR() throws Exception {
+    @Test
+    @Disabled
+    void testJvmConfigFileCR() throws Exception {
         runWithLineEndings("\r");
     }
 
@@ -53,7 +56,7 @@ public void disabledJvmConfigFileCR() throws Exception {
      * @throws Exception in case of failure
      */
     @Test
-    public void testJvmConfigFileLF() throws Exception {
+    void testJvmConfigFileLF() throws Exception {
         runWithLineEndings("\n");
     }
 
@@ -63,7 +66,7 @@ public void testJvmConfigFileLF() throws Exception {
      * @throws Exception in case of failure
      */
     @Test
-    public void testJvmConfigFileCRLF() throws Exception {
+    void testJvmConfigFileCRLF() throws Exception {
         runWithLineEndings("\r\n");
     }
 

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

@@ -101,6 +101,8 @@ public TestSuiteOrdering() {
          * the tests are to finishing. Newer tests are also more likely to fail, so this is
          * a fail fast technique as well.
          */
+        suite.addTestSuite(MavenITmng4559MultipleJvmArgsTest.class);
+        suite.addTestSuite(MavenITmng4559SpacesInJvmOptsTest.class);
         suite.addTestSuite(MavenITmng8598JvmConfigSubstitutionTest.class);
         suite.addTestSuite(MavenITmng8653AfterAndEachPhasesWithConcurrentBuilderTest.class);
         suite.addTestSuite(MavenITmng5668AfterPhaseExecutionTest.class);

its/core-it-suite/src/test/resources/mng-4559-multiple-jvm-args/pom.xml

@@ -0,0 +1,58 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>org.apache.maven.its.mng4559</groupId>
+  <artifactId>multiple-jvm-args</artifactId>
+  <version>1.0-SNAPSHOT</version>
+
+  <description>Test that multiple JVM arguments in .mvn/jvm.config are properly handled</description>
+
+  <properties>
+    <pom.test.prop1>${test.prop1}</pom.test.prop1>
+    <pom.test.prop2>${test.prop2}</pom.test.prop2>
+  </properties>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.its.plugins</groupId>
+        <artifactId>maven-it-plugin-expression</artifactId>
+        <version>2.1-SNAPSHOT</version>
+        <executions>
+          <execution>
+            <id>test</id>
+            <goals>
+              <goal>eval</goal>
+            </goals>
+            <phase>validate</phase>
+            <configuration>
+              <outputFile>target/jvm.properties</outputFile>
+              <expressions>
+                <expression>project/properties</expression>
+              </expressions>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+</project>

its/core-it-suite/src/test/resources/mng-4559-spaces-jvm-opts/.mvn/jvm.config

@@ -0,0 +1,2 @@
+# One comment
+-Dprop.jvm-opts="foo bar"