fix(maven-plugin): escape userProperties using double quotes

[xloouis]

Motivation

Fix sofa-ark-maven-plugin repackage blank space handling

Result

Resolved or fixed #1095

Summary by CodeRabbit

• Bug Fixes
  • Improved command generation for dependency analysis to correctly quote user-defined properties. Values containing spaces or special characters are now preserved, preventing failures or misinterpretation during execution. This ensures more reliable behavior when passing custom properties via command-line flags.

[sofastack-cla]

Hi @xloouis, welcome to SOFAStack community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

[coderabbitai]

Walkthrough

The plugin’s RepackageMojo now wraps Maven user property values in double quotes when constructing the dependency:tree command, ensuring values with spaces or special characters are preserved. No other logic or control flow was modified.

Changes

Cohort / File(s)	Summary
Maven tree command quoting sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/RepackageMojo.java	In doGetAllArtifactByMavenTree, user property values are now formatted as -Dkey="value" instead of -Dkey=value to handle spaces/special characters in command generation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User/IDEA
  participant Mojo as RepackageMojo
  participant MVN as Maven CLI (dependency:tree)

  U->>Mojo: Execute repackage
  Mojo->>Mojo: Collect userProperties
  Note right of Mojo: Format args as -Dkey="value"
  Mojo->>MVN: Run dependency:tree with quoted properties
  MVN-->>Mojo: Output dependency tree
  Mojo-->>U: Continue packaging flow

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nudge the flags with careful paws,
Wrap spaces snug in quoting laws.
No goals named “IDEA” run astray—
The tree now grows the proper way.
Thump-thump, I hop through build-time night,
A rabbit pleased: the args are right. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly and accurately describes the primary change by stating that the maven-plugin fix involves escaping userProperties with double quotes, which matches the change in command line generation logic.
Linked Issues Check	✅ Passed	The code now wraps all userProperties values in double quotes within doGetAllArtifactByMavenTree, directly addressing issue #1095’s requirement to preserve spaces in JVM property values and prevent Maven from misinterpreting parts of the path as lifecycle phases.
Out of Scope Changes Check	✅ Passed	All changes are confined to the quoting logic for userProperties in RepackageMojo and there are no modifications outside the scope of fixing blank-space handling in command line arguments.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/RepackageMojo.java (1)

471-516: Add unit tests and verify cross-platform quoting.

• Add a test in RepackageMojoTest covering doGetAllArtifactByMavenTree with userProperties values containing spaces, asserting the generated goals include properly quoted -Dkey="value with spaces".
• Manually verify the logged Maven command on macOS, Windows, and Linux to ensure correct quoting.

🧹 Nitpick comments (1)

sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/RepackageMojo.java (1)

487-487: Fix addresses the reported issue with spaces in property values.

Wrapping user property values in double quotes correctly prevents Maven from misinterpreting spaces as argument separators, which resolves issue #1095 where IDEA's maven.ext.class.path property caused failures.

Optional enhancement: Handle values containing double quotes.

If a property value contains embedded double quotes (rare but possible), the resulting argument would be malformed. Consider escaping quotes within values:

-                goals.add(String.format("-D%s=%s", key, "\"" + value + "\""));
+                goals.add(String.format("-D%s=\"%s\"", key, value.toString().replace("\"", "\\\"")));

Alternatively, verify that the Maven Invoker API handles quote escaping automatically when constructing the command line.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c008301 and 9969400.

📒 Files selected for processing (1)

• sofa-ark-parent/support/ark-maven-plugin/src/main/java/com/alipay/sofa/ark/boot/mojo/RepackageMojo.java (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: sca

[xloouis]

CLA 已经签署, 但好像没有刷新