Skip to content

[WIP] Fix compound commands on Windows#68825

Open
twangboy wants to merge 2 commits intosaltstack:3006.xfrom
twangboy:fix_cmd
Open

[WIP] Fix compound commands on Windows#68825
twangboy wants to merge 2 commits intosaltstack:3006.xfrom
twangboy:fix_cmd

Conversation

@twangboy
Copy link
Copy Markdown
Contributor

What does this PR do?

What issues does this PR fix or reference?

Fixes

Previous Behavior

Remove this section if not relevant

New Behavior

Remove this section if not relevant

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes/No

@twangboy twangboy added this to the Sulpher v3006.24 milestone Mar 17, 2026
@twangboy twangboy self-assigned this Mar 17, 2026
@twangboy twangboy requested a review from a team as a code owner March 17, 2026 19:49
@twangboy twangboy added the test:full Run the full test suite label Mar 17, 2026
@twangboy twangboy changed the title Fix cmd [WIP] Fix compound commands on Windows Mar 17, 2026
("echo \"foo 'bar'\"", "\"foo 'bar'\""),
('echo|set /p="foo" & echo|set /p="bar"', "foobar"),
('''echo "&<>[]|{}^=;!'+,`~ "''', '''"&<>[]|{}^=;!'+,`~ "'''),
# ('''echo "&<>[]|{}^=;!'+,`~ "''', '''"&<>[]|{}^=;!'+,`~ "'''),
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.

Reminder to add back a test testing these characters

Comment on lines 1347 to 1348
cmd must be double-quoted to ensure proper handling of space characters. The first opening
quote and the closing quote are stripped automatically by the Win32 API.
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.

Does this comment still apply with the change below? Is this going to break cases that were already quoting the command?

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

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants