Page MenuHomePhabricator
Files F15304144

D21051.id50154.diff
No OneTemporary
Actions

D21051.id50154.diff
View Options

diff --git a/src/future/exec/__tests__/ExecFutureTestCase.php b/src/future/exec/__tests__/ExecFutureTestCase.php
--- a/src/future/exec/__tests__/ExecFutureTestCase.php
+++ b/src/future/exec/__tests__/ExecFutureTestCase.php
@@ -127,6 +127,53 @@
$this->assertEqual(0, $err);
}
+ public function testEscaping() {
+ $inputs = array(
+ 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789',
+ '!@#$%^&*()-=_+`~,.<>/?[]{};\':"|',
+ '',
+ ' ',
+ 'x y',
+ '%PATH%',
+ '"',
+ '""',
+ 'a "b" c',
+ '\\',
+ '\\a',
+ 'a\\',
+ '\\a\\',
+ 'a\\a',
+ '\\\\',
+ '\\"',
+ );
+
+ $bin = $this->getSupportExecutable('echo');
+
+ foreach ($inputs as $input) {
+ if (!is_array($input)) {
+ $input = array($input);
+ }
+
+ list($stdout) = execx(
+ 'php -f %R -- %Ls',
+ $bin,
+ $input);
+
+ $stdout = explode("\n", $stdout);
+ $output = array();
+ foreach ($stdout as $line) {
+ $output[] = stripcslashes($line);
+ }
+
+ $this->assertEqual(
+ $input,
+ $output,
+ pht(
+ 'Arguments are preserved for input: %s',
+ implode(' ', $input)));
+ }
+ }
+
public function testReadBuffering() {
$str_len_8 = 'abcdefgh';
$str_len_4 = 'abcd';
diff --git a/src/toolset/workflow/ArcanistVersionWorkflow.php b/src/toolset/workflow/ArcanistVersionWorkflow.php
--- a/src/toolset/workflow/ArcanistVersionWorkflow.php
+++ b/src/toolset/workflow/ArcanistVersionWorkflow.php
@@ -67,14 +67,12 @@
Filesystem::readablePath($root)));
}
- // NOTE: Carefully execute these commands in a way that works on Windows
- // until T8298 is properly fixed. See PHI52.
-
- list($commit) = $repository_api->execxLocal('log -1 --format=%%H');
+ list($commit) = $repository_api->execxLocal(
+ 'log -1 --format=%s',
+ '%ct%x01%H');
$commit = trim($commit);
- list($timestamp) = $repository_api->execxLocal('log -1 --format=%%ct');
- $timestamp = trim($timestamp);
+ list($timestamp, $commit) = explode("\1", $commit);
$console->writeOut(
"%s %s (%s)\n",
diff --git a/src/xsprintf/PhutilCommandString.php b/src/xsprintf/PhutilCommandString.php
--- a/src/xsprintf/PhutilCommandString.php
+++ b/src/xsprintf/PhutilCommandString.php
@@ -6,6 +6,8 @@
private $escapingMode = false;
const MODE_DEFAULT = 'default';
+ const MODE_LINUX = 'linux';
+ const MODE_WINDOWS = 'windows';
const MODE_POWERSHELL = 'powershell';
public function __construct(array $argv) {
@@ -46,9 +48,19 @@
}
public static function escapeArgument($value, $mode) {
+ if ($mode === self::MODE_DEFAULT) {
+ if (phutil_is_windows()) {
+ $mode = self::MODE_WINDOWS;
+ } else {
+ $mode = self::MODE_LINUX;
+ }
+ }
+
switch ($mode) {
- case self::MODE_DEFAULT:
- return escapeshellarg($value);
+ case self::MODE_LINUX:
+ return self::escapeLinux($value);
+ case self::MODE_WINDOWS:
+ return self::escapeWindows($value);
case self::MODE_POWERSHELL:
return self::escapePowershell($value);
default:
@@ -82,4 +94,78 @@
return '"'.$value.'"';
}
+ private static function escapeLinux($value) {
+ if (strpos($value, "\0") !== false) {
+ throw new Exception(
+ pht(
+ 'Command string argument includes a NULL byte. This byte can not '.
+ 'be safely escaped in command line arguments in Linux '.
+ 'environments.'));
+ }
+
+ // If the argument is nonempty and contains only common printable
+ // characters, we do not need to escape it. This makes debugging
+ // workflows a little more user-friendly by making command output
+ // more readable.
+ if (preg_match('(^[a-zA-Z0-9:/@._+-]+\z)', $value)) {
+ return $value;
+ }
+
+ return escapeshellarg($value);
+ }
+
+ private static function escapeWindows($value) {
+ if (strpos($value, "\0") !== false) {
+ throw new Exception(
+ pht(
+ 'Command string argument includes a NULL byte. This byte can not '.
+ 'be safely escaped in command line arguments in Windows '.
+ 'environments.'));
+ }
+
+ if (!phutil_is_utf8($value)) {
+ throw new Exception(
+ pht(
+ 'Command string argument includes text which is not valid UTF-8. '.
+ 'This library can not safely escape this sequence in command '.
+ 'line arguments in Windows environments.'));
+ }
+
+ $has_backslash = (strpos($value, '\\') !== false);
+ $has_space = (strpos($value, ' ') !== false);
+ $has_quote = (strpos($value, '"') !== false);
+ $is_empty = (strlen($value) === 0);
+
+ // If a backslash appears before another backslash, a double quote, or
+ // the end of the argument, we must escape it. Otherwise, we must leave
+ // it unescaped.
+
+ if ($has_backslash) {
+ $value_v = preg_split('//', $value, -1, PREG_SPLIT_NO_EMPTY);
+ $len = count($value_v);
+ for ($ii = 0; $ii < $len; $ii++) {
+ if ($value_v[$ii] === '\\') {
+ if ($ii + 1 < $len) {
+ $next = $value_v[$ii + 1];
+ } else {
+ $next = null;
+ }
+
+ if ($next === '"' || $next === '\\' || $next === null) {
+ $value_v[$ii] = '\\\\';
+ }
+ }
+ }
+ $value = implode('', $value_v);
+ }
+
+ // Then, escape double quotes by prefixing them with backslashes.
+ if ($has_quote || $has_space || $has_backslash || $is_empty) {
+ $value = addcslashes($value, '"');
+ $value = '"'.$value.'"';
+ }
+
+ return $value;
+ }
+
}

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 7, 1:26 AM (37 m, 1 s ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7312407
Default Alt Text
D21051.id50154.diff (5 KB)

Event Timeline

Phabricator · Built by Phacility