Skip to content

Fix mvn script expanding ${...} in CLI arguments#11983

Open
ascheman wants to merge 1 commit intoapache:masterfrom
aschemaven:fix/mvn-script-eval-expansion
Open

Fix mvn script expanding ${...} in CLI arguments#11983
ascheman wants to merge 1 commit intoapache:masterfrom
aschemaven:fix/mvn-script-eval-expansion

Conversation

@ascheman
Copy link
Copy Markdown
Contributor

Summary

The eval in the mvn script causes shell expansion of ${...} patterns
in user-provided CLI arguments, breaking any argument that contains Maven
property placeholders like ${surefire.threadNumber} or ${project.basedir}.

Problem

The current script concatenates user arguments into a command string and
then uses eval exec which re-parses the string and triggers shell variable
expansion:

for arg in "$@"; do
    cmd="$cmd \"$arg\""
done
eval exec "$cmd"

Maven 3's mvn script uses exec ... "$@" which passes arguments verbatim.

Fix

Pass user arguments directly via "$@" instead of concatenating them into
the eval string. Only the base command (containing $MAVEN_OPTS etc.) uses
eval for word splitting:

eval exec "$cmd" '"$@"'

Verification

Tested locally with Maven 4.0.0-rc-5:

  • ${...} in -D arguments: no longer causes bad substitution
  • MAVEN_OPTS with spaces: still works (word splitting via eval)
  • Arguments with spaces: still works ("$@" preserves quoting)
  • 20 maven-surefire integration tests that previously failed with
    bad substitution now run successfully

Fixes #11978
Related: apache/maven-surefire#3345

Note: The same fix applies to the maven-4.0.x branch where the mvn
script is identical.

The eval in the mvn script causes shell expansion of ${...} patterns
in user-provided arguments. Pass user arguments directly via "$@"
instead of concatenating them into the eval string. This preserves
MAVEN_OPTS word splitting while preventing unintended shell expansion.

Fixes apache#11978

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hboutemy
Copy link
Copy Markdown
Member

I see that the code was introduced in #2213
are you clear that the new approach does not break the intent in #2213 (I took time to search when the code was done, I have absolute no clue: that's why I'm asking)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mvn script eval expands ${...} in CLI arguments

2 participants