Skip to content

fix: PowerShell quoting for special chars and -D property spacing in app install#2585

Merged
maxandersen merged 2 commits into
mainfrom
fix/windows-quoting-and-sysprop-spacing
Jul 5, 2026
Merged

fix: PowerShell quoting for special chars and -D property spacing in app install#2585
maxandersen merged 2 commits into
mainfrom
fix/windows-quoting-and-sysprop-spacing

Conversation

@maxandersen

@maxandersen maxandersen commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

turns out we had "bad" quoting rules on windows since 2020 :)

-Dwhatever.period=true breaks on powershell, works on cmd.exe

and since 2024 our install scripts generated -D x=y instead of -Dx=y causing issues too.

This fixes both.

1. PowerShell safe chars too permissive (#2584)

pwrSafeChars included .,;:() as safe characters, inherited from original best-guess allowlist (47b9f34, 2020). These characters cause issues in PowerShell — args like -Djavafx.preview=true were passed through unquoted and got mangled.

Fix: Remove .,;:() from pwrSafeChars. Args containing these chars now get single-quoted in PowerShell (e.g. '-Djavafx.preview=true'). CMD is unchanged — these chars are safe there.

2. app install -Dkey=value splits -D from property (#2564)

DependencyInfoMixin.opts() emitted -D and key=value as two separate list elements. The generated wrapper script ended up with -D camel.jbang.version=4.21.0 (space-separated), which jbang run then rejects with "Option -D, must be part of a property".

Fix: Join them: opts.add("-D" + key + "=" + value).

Tests added

  • Unit: TestCommandBuffer — PowerShell escapes .,;:(), CMD does not
  • Unit: TestApp.testAppInstallWithSystemProperty — verifies generated scripts contain -Dkey=value (no space)
  • Integration: AppIT.shouldKeepSystemPropertyAttached — end-to-end: app install -Djavafx.preview=true runs and JVM receives the property intact

Fixes #2584
Fixes #2564

@maxandersen
fix: PowerShell quoting for dots/commas/colons and -D property spacin…
7038239 
…g in app install

- Remove .,;:() from PowerShell safe chars so args like -Djavafx.preview=true
  get single-quoted instead of passed through bare (fixes #2584)
- Join -D with property key in DependencyInfoMixin.opts() so app install
  generates -Dkey=value instead of -D key=value (fixes #2564)
@coderabbitai

coderabbitai Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • ai-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5cdf71fa-44c2-4155-9d01-2e97853c7b83

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

Comment @coderabbitai help to get the list of available commands.

@maxandersen maxandersen requested a review from quintesse July 5, 2026 08:41
maxandersen
maxandersen commented Jul 5, 2026
View reviewed changes
Comment thread src/main/java/dev/jbang/util/CommandBuffer.java
static Pattern cmdSafeChars = Pattern.compile("[a-zA-Z0-9.,_+=:;@()-\\\\]*");
// TODO: Figure out what the real list of safe characters is for PowerShell
static Pattern pwrSafeChars = Pattern.compile("[a-zA-Z0-9.,_+=:@()\\\\-]*");
static Pattern pwrSafeChars = Pattern.compile("[a-zA-Z0-9_+=@\\\\-]*");

@maxandersen maxandersen Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is tricky is that

jbang com.h2database:h2:1.4.200

is fine, but

jbang -Dsdff=sdfd.df com.h2database:h2:1.4.200

is not. the kicker is - needs more quoting than regular arguments.

for now let them be quoted (meaning on windows installed scripts will quote the arguments with GAV.

might want to refine it in future .

quintesse
quintesse approved these changes Jul 5, 2026
View reviewed changes

@quintesse quintesse left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@maxandersen maxandersen merged commit 4bb8916 into main Jul 5, 2026
30 checks passed
@maxandersen maxandersen deleted the fix/windows-quoting-and-sysprop-spacing branch July 5, 2026 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@quintesse quintesse quintesse approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

2 participants

@maxandersen @quintesse