Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

escape the '&' char in windows links #7960

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zangold
Copy link

@zangold zangold commented May 8, 2024

The & character is special in windows, so escape it when passing strings that contain '&' as arguments to cmd.exe.

This fixes a bug where ctrl-clicking on a link with multiple parameters would only open the substring of the link up to (and not including) the first ampersand, e.g.,

http://www.foo.bar/index.html?foo=bar&baz=wat

would direct me to:

http://www.foo.bar/index.html?foo=bar

The & character is special in windows, so escape it when passing
strings that contain '&' as arguments to cmd.exe.

This fixes a bug where ctrl-clicking on a link with multiple
parameters would only open the substring of the link up to (and not
including) the first ampersand, e.g.,

http://www.foo.bar/index.html?foo=bar&baz=wat

would direct me to:

http://www.foo.bar/index.html?foo=bar
@chrisduerr
Copy link
Member

Are you suggesting this is something inherent to spawning any command on Windows? Because I don't see how that would be true.

@zangold
Copy link
Author

zangold commented May 8, 2024

I'm having difficulty finding any official documentation from microsoft on this, but some stackoverflow answers seem to suggest that cmd.exe uses '&' as a way to specify multiple commands on the same line, equivalent to putting a semicolon in a line in bash.

On further review, it seems as though delimiting the URL with double quotes also fixes this issue:

  • start "" http://www.foo.bar/index.html?foo=bar&baz=wat opens the shortened link (and prints a message about baz not being a valid command)
  • start "" "http://www.foo.bar/index.html?foo=bar&baz=wat" opens the full link, without the above error message about baz

I can update the PR to quote-delimit the URL instead of escaping the & character, if you prefer.

@chrisduerr
Copy link
Member

Commands launched by hints don't have to execute cmd.exe, it can be executed by any command. Your PR doesn't make sense to me.

@zangold
Copy link
Author

zangold commented May 8, 2024

I think you might be right that there is a better approach to this -- per the Rust docs on std::process, it sounds like passing args into CreateProcessW is janky, at best: https://doc.rust-lang.org/std/process/index.html#windows-argument-splitting

It's worth noting that, because ampersands aren't correctly escaped in the daemon processes that alacritty launches, this is actually a security concern -- I think one would be able to construct some text with hyperlink terminal directives that can execute an arbitrary command on the user's computer if they control-click on it.

I'll see if I can come up with a solution that fixes the control-click behaviour and tries to sanitize the args passed into CreateProcessW.

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

Successfully merging this pull request may close these issues.

None yet

2 participants