Changeset View
Changeset View
Standalone View
Standalone View
src/xsprintf/PhutilCommandString.php
| <?php | <?php | ||||
| final class PhutilCommandString extends Phobject { | final class PhutilCommandString extends Phobject { | ||||
| private $argv; | private $argv; | ||||
| private $escapingMode = false; | private $escapingMode = false; | ||||
| const MODE_DEFAULT = 'default'; | const MODE_DEFAULT = 'default'; | ||||
| const MODE_POWERSHELL = 'powershell'; | const MODE_POWERSHELL = 'powershell'; | ||||
| public function __construct(array $argv) { | public function __construct(array $argv) { | ||||
| $this->argv = $argv; | $this->argv = $argv; | ||||
| $this->escapingMode = self::MODE_DEFAULT; | $this->escapingMode = phutil_is_windows() ? self::MODE_POWERSHELL : self::MODE_DEFAULT; | ||||
| // This makes sure we throw immediately if there are errors in the | // This makes sure we throw immediately if there are errors in the | ||||
| // parameters. | // parameters. | ||||
| $this->getMaskedString(); | $this->getMaskedString(); | ||||
| } | } | ||||
| public function __toString() { | public function __toString() { | ||||
| return $this->getMaskedString(); | return $this->getMaskedString(); | ||||
| Show All 20 Lines | return xsprintf( | ||||
| 'mode' => $this->escapingMode, | 'mode' => $this->escapingMode, | ||||
| ), | ), | ||||
| $this->argv); | $this->argv); | ||||
| } | } | ||||
| public static function escapeArgument($value, $mode) { | public static function escapeArgument($value, $mode) { | ||||
| switch ($mode) { | switch ($mode) { | ||||
| case self::MODE_DEFAULT: | case self::MODE_DEFAULT: | ||||
| return escapeshellarg($value); | return escapeshellarg($value); | ||||
hach-que: There should be `MODE_WIN_CMD` to force Windows CMD escaping for e.g. when Drydock is executing… | |||||
Not Done Inline ActionsGood point. That said where should I change to make Drydock use that mode? Also in that case does MODE_POWERSHELL become obsolete? BYK: Good point. That said where should I change to make Drydock use that mode? Also in that case… | |||||
Not Done Inline ActionsNo need to change Drydock. Adding a specific mode will just future proof this code. hach-que: No need to change Drydock. Adding a specific mode will just future proof this code. | |||||
Not Done Inline ActionsDropping CMD now (apart from the other breakage) will mean we have to come back and add it later, because you can't remotely execute without cmd. hach-que: Dropping CMD now (apart from the other breakage) will mean we have to come back and add it… | |||||
Not Done Inline Actions
I'm not sure if I understand what is behind this. Can you elaborate? BYK: > because you can't remotely execute without cmd.
I'm not sure if I understand what is behind… | |||||
| case self::MODE_POWERSHELL: | case self::MODE_POWERSHELL: | ||||
| return self::escapePowershell($value); | return self::escapePowershell($value); | ||||
| default: | default: | ||||
| throw new Exception(pht('Unknown escaping mode!')); | throw new Exception(pht('Unknown escaping mode!')); | ||||
| } | } | ||||
| } | } | ||||
| private static function escapePowershell($value) { | private static function escapePowershell($value) { | ||||
| Show All 26 Lines | |||||
There should be MODE_WIN_CMD to force Windows CMD escaping for e.g. when Drydock is executing a command on a remote Windows host (in this scenario, phutil_is_windows returns false because the Phabricator host is Linux).