Skip to content

Commit d5801bc

Browse files
BobVulgnodet
BobVul
authored and
gnodet
committed
Avoid parsing MAVEN_OPTS (master/4.x) (apache#10970)
Fixes apache#10937 by introducing an additional INTERNAL_MAVEN_OPTS for any arguments that need to be inserted by the script. Parsing the externally-defined MAVEN_OPTS variable can lead to incorrect processing of quotes and special characters, so use the separate variable to avoid doing so. Additionally JVM_CONFIG_MAVEN_OPTS is introduced as its own variable to preserve the append behaviour. Remove quotes from the new JVM_CONFIG_MAVEN_OPTS to also allow quoted pipes to work from jvm.config This is a follow-up to apache#10937, where the extra layer of quotes causes parsing issues in Windows cmd Test that adding pipes to either MAVEN_OPTS or jvm.config does not break anything Note: it is important that a jvm.config exists for the MAVEN_OPTS portion of the test to work By default xargs handles quotes specially. To avoid this behaviour, `-0` must be used instead, but first we need to convert LF to NUL. Since quotes are no longer being stripped by xargs, we should also stop trying to add them back in otherwise nested quotes cause further issues --------- Co-authored-by: Bob <BobVul@users.noreply.github.com>
1 parent 47f7422 commit d5801bc

File tree

6 files changed

+130
-9
lines changed

6 files changed

+130
-9
lines changed

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,20 +171,18 @@ concat_lines() {
171171
# First convert all CR to LF using tr
172172
tr '\r' '\n' < "$1" | \
173173
sed -e '/^$/d' -e 's/#.*$//' | \
174+
# Replace LF with NUL for xargs
175+
tr '\n' '\0' | \
174176
# Split into words and process each argument
175-
xargs -n 1 | \
177+
# Use -0 with NUL to avoid special behaviour on quotes
178+
xargs -n 1 -0 | \
176179
while read -r arg; do
177180
# Replace variables first
178181
arg=$(echo "$arg" | sed \
179182
-e "s@\${MAVEN_PROJECTBASEDIR}@$MAVEN_PROJECTBASEDIR@g" \
180183
-e "s@\$MAVEN_PROJECTBASEDIR@$MAVEN_PROJECTBASEDIR@g")
181184

182-
# Add quotes only if argument contains spaces and isn't already quoted
183-
if echo "$arg" | grep -q " " && ! echo "$arg" | grep -q "^\".*\"$"; then
184-
echo "\"$arg\""
185-
else
186-
echo "$arg"
187-
fi
185+
echo "$arg"
188186
done | \
189187
tr '\n' ' '
190188
fi

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ title %0
3535
@REM enable echoing by setting MAVEN_BATCH_ECHO to 'on'
3636
@if "%MAVEN_BATCH_ECHO%"=="on" echo %MAVEN_BATCH_ECHO%
3737

38+
@REM Clear/define a variable for any options to be inserted via script
39+
@REM We want to avoid trying to parse the external MAVEN_OPTS variable
40+
SET INTERNAL_MAVEN_OPTS=
41+
3842
@REM Execute a user defined script before this one
3943
if not "%MAVEN_SKIP_RC%"=="" goto skipRc
4044
if exist "%PROGRAMDATA%\mavenrc.cmd" call "%PROGRAMDATA%\mavenrc.cmd" %*
@@ -204,7 +208,7 @@ for /F "usebackq tokens=* delims=" %%a in ("%MAVEN_PROJECTBASEDIR%\.mvn\jvm.conf
204208
)
205209
)
206210
)
207-
@endlocal & set "MAVEN_OPTS=%MAVEN_OPTS% %JVM_CONFIG_MAVEN_OPTS%"
211+
@endlocal & set JVM_CONFIG_MAVEN_OPTS=%JVM_CONFIG_MAVEN_OPTS%
208212

209213
:endReadJvmConfig
210214

@@ -224,7 +228,7 @@ if "%~1"=="--debug" (
224228
echo Error: Unable to autodetect the YJP library location. Please set YJPLIB variable >&2
225229
exit /b 1
226230
)
227-
set "MAVEN_OPTS=-agentpath:%YJPLIB%=onexit=snapshot,onexit=memory,tracing,onlylocal %MAVEN_OPTS%"
231+
set "INTERNAL_MAVEN_OPTS=-agentpath:%YJPLIB%=onexit=snapshot,onexit=memory,tracing,onlylocal %INTERNAL_MAVEN_OPTS%"
228232
) else if "%~1"=="--enc" (
229233
set "MAVEN_MAIN_CLASS=org.apache.maven.cling.MavenEncCling"
230234
) else if "%~1"=="--shell" (
@@ -248,7 +252,9 @@ set LAUNCHER_CLASS=org.codehaus.plexus.classworlds.launcher.Launcher
248252
if "%MAVEN_MAIN_CLASS%"=="" @set MAVEN_MAIN_CLASS=org.apache.maven.cling.MavenCling
249253

250254
"%JAVACMD%" ^
255+
%INTERNAL_MAVEN_OPTS% ^
251256
%MAVEN_OPTS% ^
257+
%JVM_CONFIG_MAVEN_OPTS% ^
252258
%MAVEN_DEBUG_OPTS% ^
253259
--enable-native-access=ALL-UNNAMED ^
254260
-classpath %LAUNCHER_JAR% ^

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
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.nio.file.Path;
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/10937">gh-10937</a>.
30+
*/
31+
class MavenITgh10937QuotedPipesInMavenOptsTest extends AbstractMavenIntegrationTestCase {
32+
33+
MavenITgh10937QuotedPipesInMavenOptsTest() {
34+
super("[3.0.0,)");
35+
}
36+
37+
/**
38+
* Verify the dependency management of the consumer POM is computed correctly
39+
*/
40+
@Test
41+
void testIt() throws Exception {
42+
Path basedir =
43+
extractResources("/gh-10937-pipes-maven-opts").getAbsoluteFile().toPath();
44+
45+
Verifier verifier = newVerifier(basedir.toString());
46+
verifier.setEnvironmentVariable("MAVEN_OPTS", "-Dprop.maven-opts=\"foo|bar\"");
47+
verifier.addCliArguments("validate");
48+
verifier.execute();
49+
verifier.verifyErrorFreeLog();
50+
51+
Properties props = verifier.loadProperties("target/pom.properties");
52+
assertEquals("foo|bar", props.getProperty("project.properties.pom.prop.jvm-opts"));
53+
assertEquals("foo|bar", props.getProperty("project.properties.pom.prop.maven-opts"));
54+
}
55+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ public TestSuiteOrdering() {
103103
* the tests are to finishing. Newer tests are also more likely to fail, so this is
104104
* a fail fast technique as well.
105105
*/
106+
suite.addTestSuite(MavenITgh10937QuotedPipesInMavenOptsTest.class);
106107
suite.addTestSuite(MavenITgh2532DuplicateDependencyEffectiveModelTest.class);
107108
suite.addTestSuite(MavenITmng8736ConcurrentFileActivationTest.class);
108109
suite.addTestSuite(MavenITmng8744CIFriendlyTest.class);

‎its/core-it-suite/src/test/resources/gh-10937-pipes-maven-opts/.mvn/jvm.config‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# One comment
2+
-Dprop.jvm-opts="foo|bar"

‎its/core-it-suite/src/test/resources/gh-10937-pipes-maven-opts/pom.xml‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
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" 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">
21+
<modelVersion>4.0.0</modelVersion>
22+
23+
<groupId>org.apache.maven.its.gh10937</groupId>
24+
<artifactId>test</artifactId>
25+
<version>1.0</version>
26+
27+
<name>Maven Integration Test :: GH-10937</name>
28+
<description>Verify that JVM args can contain pipes.</description>
29+
30+
<properties>
31+
<pom.prop.maven-opts>${prop.maven-opts}</pom.prop.maven-opts>
32+
<pom.prop.jvm-opts>${prop.jvm-opts}</pom.prop.jvm-opts>
33+
</properties>
34+
35+
<build>
36+
<plugins>
37+
<plugin>
38+
<groupId>org.apache.maven.its.plugins</groupId>
39+
<artifactId>maven-it-plugin-expression</artifactId>
40+
<version>2.1-SNAPSHOT</version>
41+
<executions>
42+
<execution>
43+
<id>test</id>
44+
<goals>
45+
<goal>eval</goal>
46+
</goals>
47+
<phase>validate</phase>
48+
<configuration>
49+
<outputFile>target/pom.properties</outputFile>
50+
<expressions>
51+
<expression>project/properties</expression>
52+
</expressions>
53+
</configuration>
54+
</execution>
55+
</executions>
56+
</plugin>
57+
</plugins>
58+
</build>
59+
</project>

0 commit comments

Comments
 (0)