slachiewicz commented on PR #434: URL: https://github.com/apache/maven-shared-utils/pull/434#issuecomment-5632504313
For reviewers: how plexus-utils handles the same problem, since its `CmdShell` is the sibling of this one. **Behaviour.** plexus-utils [`Shell.quoteOneItem`](https://github.com/codehaus-plexus/plexus-utils/blob/master/src/main/java/org/codehaus/plexus/util/cli/shell/Shell.java) goes through `quoteAndEscape` with a single trigger, the space: an item with a space gets double quotes, an already-quoted item is left alone, nothing is escaped by default. That is what shared-utils 3.4.2 did before ef89c5a removed the path. This PR restores that and adds the `cmd.exe /?` specials `& < > ( ) @ ^ |` as triggers; plexus-utils has no equivalent, so `lol(1)` and `a&b` still fail there. **Reported there, never fixed by quoting.** [#37](https://github.com/codehaus-plexus/plexus-utils/issues/37) (asterisk in a password) has been open since 2018; [#36](https://github.com/codehaus-plexus/plexus-utils/issues/36) (embedded double quote) was closed as its duplicate. The maintainers' direction is architectural: [PR #109](https://github.com/codehaus-plexus/plexus-utils/pull/109) drops the `cmd.exe` wrapper and hands the argument vector to `ProcessBuilder`, and #37 says explicitly that the shell implementations should be replaced rather than taught more escaping. That PR has been open since 2021. **What this PR shares with plexus-utils, on purpose.** An embedded `"` is not escaped and `*` is not quoted, because `cmd.exe` toggles quote state on every `"` and no escaping survives it reliably. Those cases belong with the "drop the shell" change, not with trigger characters. One small divergence: under unconditional quoting plexus-utils re-wraps an already-quoted item, this PR leaves it alone. **Follow-up, separate from this PR.** plexus-utils runs `cmd.exe /X /D /C` since [9093271](https://github.com/codehaus-plexus/plexus-utils/commit/9093271) (2024, for MJAVADOC-811): `/D` skips the Command Processor AutoRun registry entry, which in some corporate setups changes the working directory of every spawned `cmd.exe`. Our `CmdShell` still runs `/X /C` and has the same exposure. *This comment was created with AI assistance.* -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
