Skip to content

[Fix] Escape backslashes in PowerShell ArgumentList #3658

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

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

CommandMC
Copy link
Collaborator

@CommandMC CommandMC commented Mar 31, 2024

Backslashes are mostly fine to just include without escaping, but there are certain cases where they can cause issues. One of those would be when specifying just a drive letter (X:\) as one argument (for example as the install path for a game). In that case, when adding the necessary escape characters around the argument, we'd end up with "`"X:\`"". PS would then see that the backtick is escaped by a \ (even though it's not supposed to be), and the whole thing would fall apart

Resolving this is luckily rather easy, we can simply replace every one \ with two \ to end up with one escaped \, which can then no longer break anything around it

To test:

  • Be on Windows
  • Try to install a game onto a "bare" drive letter (for example D:\)
  • Won't work on main, will work here

This still needs a bit more testing with uncommon arguments, PS is pretty unpredictable after all


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@CommandMC CommandMC added the pr:testing This PR is in testing, don't merge. label Mar 31, 2024
@CommandMC CommandMC self-assigned this Mar 31, 2024
@CommandMC CommandMC added pr:ready-for-review Feature-complete, ready for the grind! :P and removed pr:testing This PR is in testing, don't merge. labels Mar 31, 2024
Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

Tested here and works fine. Nice job!

@CommandMC CommandMC merged commit 4f3d892 into main Apr 19, 2024
9 checks passed
@CommandMC CommandMC deleted the fix/install-to-bare-drive branch April 19, 2024 08:56
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Apr 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants