Ref T2015. This adds support for explicitly escaping commands for the Windows Command Prompt or Bash in PhutilCommandString.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T2015: Implement Drydock
Tested as part of a diff (not yet submitted) to support different shells in Harbormaster / Drydock.
Diff Detail
Diff Detail
- Repository
- rPHU libphutil
- Branch
- alternate-shell
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 2633 Build 2637: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
The windows case is obviously broken. I'd like to stop introducing any more broken Windows escaping.
The bash case looks broken and unsafe for, offhand, string containing backslashes.
It's not clear to me why the bash escaping needs to differ from escapeshellarg(). I would expect this to be correct for bash, as it is routinely used to escape things in bash.
Comment Actions
The windows case is obviously broken. I'd like to stop introducing any more broken Windows escaping.
This should replace any other Windows escaping code (for example, with this, the hacks for mkdir in host leasing just get replaced with this).
The bash case looks broken and unsafe for, offhand, string containing backslashes.
Backslashes don't do anything for single quoted strings in Bash. Since we encapsulate the whole string in single quotes, and then move individual single quote characters into their own quote context, it should prevent any backslash escaping. For example:
\' TEST
becomes
'\'"'"' TEST'
which under Bash shows:
june-linux-laptop:~> echo '\'"'"' TEST' \' TEST
It's not clear to me why the bash escaping needs to differ from escapeshellarg(). I would expect this to be correct for bash, as it is routinely used to escape things in bash.
escapeshellarg has different behaviour on Windows (it attempts to escape for CMD, not for Bash).