Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14074883
D21509.id51198.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D21509.id51198.diff
View Options
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;
+}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Nov 22, 9:07 AM (4 h, 35 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6774193
Default Alt Text
D21509.id51198.diff (12 KB)
Attached To
Mode
D21509: Provide "gitsprintf(...)" and disambiguate Git ref selectors
Attached
Detach File
Event Timeline
Log In to Comment