diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -762,6 +762,7 @@
'PhutilGitHubResponse' => 'future/github/PhutilGitHubResponse.php',
'PhutilGitURI' => 'parser/PhutilGitURI.php',
'PhutilGitURITestCase' => 'parser/__tests__/PhutilGitURITestCase.php',
+ 'PhutilGitsprintfTestCase' => 'xsprintf/__tests__/PhutilGitsprintfTestCase.php',
'PhutilHTMLParser' => 'parser/html/PhutilHTMLParser.php',
'PhutilHTMLParserTestCase' => 'parser/html/__tests__/PhutilHTMLParserTestCase.php',
'PhutilHTTPEngineExtension' => 'future/http/PhutilHTTPEngineExtension.php',
@@ -927,6 +928,7 @@
'csprintf' => 'xsprintf/csprintf.php',
'exec_manual' => 'future/exec/execx.php',
'execx' => 'future/exec/execx.php',
+ 'gitsprintf' => 'xsprintf/gitsprintf.php',
'head' => 'utils/utils.php',
'head_key' => 'utils/utils.php',
'hgsprintf' => 'xsprintf/hgsprintf.php',
@@ -1043,6 +1045,7 @@
'xsprintf' => 'xsprintf/xsprintf.php',
'xsprintf_callback_example' => 'xsprintf/xsprintf.php',
'xsprintf_command' => 'xsprintf/csprintf.php',
+ 'xsprintf_git' => 'xsprintf/gitsprintf.php',
'xsprintf_javascript' => 'xsprintf/jsprintf.php',
'xsprintf_ldap' => 'xsprintf/ldapsprintf.php',
'xsprintf_mercurial' => 'xsprintf/hgsprintf.php',
@@ -1825,6 +1828,7 @@
'PhutilGitHubResponse' => 'Phobject',
'PhutilGitURI' => 'Phobject',
'PhutilGitURITestCase' => 'PhutilTestCase',
+ 'PhutilGitsprintfTestCase' => 'PhutilTestCase',
'PhutilHTMLParser' => 'Phobject',
'PhutilHTMLParserTestCase' => 'PhutilTestCase',
'PhutilHTTPEngineExtension' => 'Phobject',
diff --git a/src/land/engine/ArcanistGitLandEngine.php b/src/land/engine/ArcanistGitLandEngine.php
--- a/src/land/engine/ArcanistGitLandEngine.php
+++ b/src/land/engine/ArcanistGitLandEngine.php
@@ -274,9 +274,11 @@
// as changes.
list($changes) = $api->execxLocal(
- 'diff --no-ext-diff %s..%s --',
- $into_commit,
- $max_hash);
+ 'diff --no-ext-diff %s --',
+ gitsprintf(
+ '%s..%s',
+ $into_commit,
+ $max_hash));
$changes = trim($changes);
if (!strlen($changes)) {
@@ -762,7 +764,7 @@
$api = $this->getRepositoryAPI();
list($stdout) = $api->execxLocal(
- 'merge-base %s %s',
+ 'merge-base -- %s %s',
$branch,
$commit);
$merge_base = trim($stdout);
@@ -781,7 +783,7 @@
list($info) = $api->execxLocal(
'log -n1 --format=%s %s --',
'%aD%n%an%n%ae',
- $commit);
+ gitsprintf('%s', $commit));
$info = trim($info);
list($date, $author, $email) = explode("\n", $info, 3);
@@ -1452,19 +1454,19 @@
$commit_map = array();
foreach ($symbols as $symbol) {
$symbol_commit = $symbol->getCommit();
- $format = '%H%x00%P%x00%s%x00';
+ $format = '--format=%H%x00%P%x00%s%x00';
if ($into_commit === null) {
list($commits) = $api->execxLocal(
- 'log %s --format=%s',
- $symbol_commit,
- $format);
+ 'log %s %s --',
+ $format,
+ gitsprintf('%s', $symbol_commit));
} else {
list($commits) = $api->execxLocal(
- 'log %s --not %s --format=%s',
- $symbol_commit,
- $into_commit,
- $format);
+ 'log %s %s --not %s --',
+ $format,
+ gitsprintf('%s', $symbol_commit),
+ gitsprintf('%s', $into_commit));
}
$commits = phutil_split_lines($commits, false);
diff --git a/src/query/ArcanistGitCommitMessageHardpointQuery.php b/src/query/ArcanistGitCommitMessageHardpointQuery.php
--- a/src/query/ArcanistGitCommitMessageHardpointQuery.php
+++ b/src/query/ArcanistGitCommitMessageHardpointQuery.php
@@ -27,7 +27,7 @@
$futures[$hash] = $api->execFutureLocal(
'log -n1 --format=%s %s --',
'%s%n%n%b',
- $hash);
+ gitsprintf('%s', $hash));
}
yield $this->yieldFutures($futures);
diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -88,7 +88,7 @@
*/
private function isDescendant($child, $parent) {
list($common_ancestor) = $this->execxLocal(
- 'merge-base %s %s',
+ 'merge-base -- %s %s',
$child,
$parent);
$common_ancestor = trim($common_ancestor);
@@ -214,7 +214,7 @@
}
list($err, $merge_base) = $this->execManualLocal(
- 'merge-base %s %s',
+ 'merge-base -- %s %s',
$symbolic_commit,
$this->getHeadCommit());
if ($err) {
@@ -381,7 +381,7 @@
}
list($merge_base) = $this->execxLocal(
- 'merge-base %s HEAD',
+ 'merge-base -- %s HEAD',
$default_relative);
return trim($merge_base);
@@ -569,22 +569,24 @@
} else if ($relative == self::GIT_MAGIC_ROOT_COMMIT) {
// First commit.
list($stdout) = $this->execxLocal(
- 'log --format=medium HEAD');
+ 'log --format=medium HEAD --');
} else {
// 2..N commits.
list($stdout) = $this->execxLocal(
- 'log --first-parent --format=medium %s..%s',
- $this->getBaseCommit(),
- $this->getHeadCommit());
+ 'log --first-parent --format=medium %s --',
+ gitsprintf(
+ '%s..%s',
+ $this->getBaseCommit(),
+ $this->getHeadCommit()));
}
return $stdout;
}
public function getGitHistoryLog() {
list($stdout) = $this->execxLocal(
- 'log --format=medium -n%d %s',
+ 'log --format=medium -n%d %s --',
self::SEARCH_LENGTH_FOR_PARENT_REVISIONS,
- $this->getBaseCommit());
+ gitsprintf('%s', $this->getBaseCommit()));
return $stdout;
}
@@ -721,7 +723,7 @@
array(
'diff %C --raw %s --',
$diff_options,
- $diff_base,
+ gitsprintf('%s', $diff_base),
));
$untracked_future = $this->buildLocalFuture(
@@ -782,7 +784,7 @@
list($stdout, $stderr) = $this->execxLocal(
'diff %C --raw %s HEAD --',
$this->getDiffBaseOptions(),
- $this->getBaseCommit());
+ gitsprintf('%s', $this->getBaseCommit()));
return $this->parseGitRawDiff($stdout);
}
@@ -935,15 +937,15 @@
public function getChangedFiles($since_commit) {
list($stdout) = $this->execxLocal(
- 'diff --raw %s',
- $since_commit);
+ 'diff --raw %s --',
+ gitsprintf('%s', $since_commit));
return $this->parseGitRawDiff($stdout);
}
public function getBlame($path) {
list($stdout) = $this->execxLocal(
'blame --porcelain -w -M %s -- %s',
- $this->getBaseCommit(),
+ gitsprintf('%s', $this->getBaseCommit()),
$path);
// the --porcelain format prints at least one header line per source line,
@@ -1030,7 +1032,7 @@
list($stdout) = $this->execxLocal(
'ls-tree %s -- %s',
- $revision,
+ gitsprintf('%s', $revision),
$path);
$info = $this->parseGitTree($stdout);
@@ -1046,7 +1048,7 @@
}
list($stdout) = $this->execxLocal(
- 'cat-file blob %s',
+ 'cat-file blob -- %s',
$info[$path]['ref']);
return $stdout;
}
@@ -1171,7 +1173,7 @@
list($message) = $this->execxLocal(
'log -n1 --format=%C %s --',
'%s%n%n%b',
- $commit);
+ gitsprintf('%s', $commit));
return $message;
}
@@ -1249,9 +1251,9 @@
}
list($summary) = $this->execxLocal(
- 'log -n 1 --format=%C %s',
- '%s',
- $commit);
+ 'log -n 1 %s %s --',
+ '--format=%s',
+ gitsprintf('%s', $commit));
return trim($summary);
}
@@ -1268,7 +1270,7 @@
$matches = null;
if (preg_match('/^merge-base\((.+)\)$/', $name, $matches)) {
list($err, $merge_base) = $this->execManualLocal(
- 'merge-base %s HEAD',
+ 'merge-base -- %s HEAD',
$matches[1]);
if (!$err) {
$this->setBaseCommitExplanation(
@@ -1282,7 +1284,7 @@
}
} else if (preg_match('/^branch-unique\((.+)\)$/', $name, $matches)) {
list($err, $merge_base) = $this->execManualLocal(
- 'merge-base %s HEAD',
+ 'merge-base -- %s HEAD',
$matches[1]);
if ($err) {
return null;
@@ -1400,7 +1402,7 @@
if (!$err) {
$upstream = rtrim($upstream);
list($upstream_merge_base) = $this->execxLocal(
- 'merge-base %s HEAD',
+ 'merge-base -- %s HEAD',
$upstream);
$upstream_merge_base = rtrim($upstream_merge_base);
$this->setBaseCommitExplanation(
diff --git a/src/repository/graph/query/ArcanistGitCommitGraphQuery.php b/src/repository/graph/query/ArcanistGitCommitGraphQuery.php
--- a/src/repository/graph/query/ArcanistGitCommitGraphQuery.php
+++ b/src/repository/graph/query/ArcanistGitCommitGraphQuery.php
@@ -84,7 +84,7 @@
$format = implode('%x02', $fields).'%x01';
$future = $api->newFuture(
- 'log --format=%s %Ls --stdin',
+ 'log --format=%s %Ls --stdin --',
$format,
$flags);
$future->write($ref_blob);
diff --git a/src/xsprintf/__tests__/PhutilGitsprintfTestCase.php b/src/xsprintf/__tests__/PhutilGitsprintfTestCase.php
new file mode 100644
--- /dev/null
+++ b/src/xsprintf/__tests__/PhutilGitsprintfTestCase.php
@@ -0,0 +1,40 @@
+<?php
+
+final class PhutilGitsprintfTestCase extends PhutilTestCase {
+
+ public function testHgsprintf() {
+ $selectors = array(
+ 'HEAD' => 'HEAD',
+ 'master' => 'master',
+ 'a..b' => 'a..b',
+ 'feature^' => 'feature^',
+ '--flag' => false,
+ );
+
+ foreach ($selectors as $input => $expect) {
+ $caught = null;
+
+ try {
+ $output = gitsprintf('%s', $input);
+ } catch (Exception $ex) {
+ $caught = $ex;
+ } catch (Throwable $ex) {
+ $caught = $ex;
+ }
+
+ if ($caught !== null) {
+ $actual = false;
+ } else {
+ $actual = $output;
+ }
+
+ $this->assertEqual(
+ $expect,
+ $actual,
+ pht(
+ 'Result for input "%s".',
+ $input));
+ }
+ }
+
+}
diff --git a/src/xsprintf/gitsprintf.php b/src/xsprintf/gitsprintf.php
new file mode 100644
--- /dev/null
+++ b/src/xsprintf/gitsprintf.php
@@ -0,0 +1,64 @@
+<?php
+
+/**
+ * Format a Git ref selector. This formatting is important when executing
+ * commands like "git log" which can not unambiguously parse all values as
+ * ref selectors.
+ *
+ * Supports the following conversions:
+ *
+ * %s Ref Selector
+ * Escapes a Git ref selector. In particular, this will reject ref selectors
+ * which Git may interpret as flags.
+ *
+ * %R Raw String
+ * Passes text through unescaped.
+ */
+function gitsprintf($pattern /* , ... */) {
+ $args = func_get_args();
+ return xsprintf('xsprintf_git', null, $args);
+}
+
+/**
+ * @{function:xsprintf} callback for Git encoding.
+ */
+function xsprintf_git($userdata, &$pattern, &$pos, &$value, &$length) {
+ $type = $pattern[$pos];
+
+ switch ($type) {
+ case 's':
+
+ // See T13589. Some Git commands accept both a ref selector and a list of
+ // paths. For example:
+
+ // $ git log <ref> -- <path> <path> ...
+
+ // These commands disambiguate ref selectors from paths using "--", but
+ // have no mechanism for disambiguating ref selectors from flags.
+
+ // Thus, there appears to be no way (in the general case) to safely
+ // invoke these commands with an arbitrary ref selector string: ref
+ // selector strings like "--flag" may be interpreted as flags, not as
+ // ref selectors.
+
+ // To resolve this, we reject any ref selector which begins with "-".
+ // These selectors are never valid anyway, so there is no loss of overall
+ // correctness. It would be more desirable to pass them to Git in a way
+ // that guarantees Git inteprets the string as a ref selector, but it
+ // appears that no mechanism exists to allow this.
+
+ if (preg_match('(^-)', $value)) {
+ throw new Exception(
+ pht(
+ 'Git ref selector "%s" is not a valid selector and can not be '.
+ 'passed to the Git CLI safely in the general case.',
+ $value));
+ }
+ break;
+ case 'R':
+ $type = 's';
+ break;
+ }
+
+ $pattern[$pos] = $type;
+}