From ea9cb0b625fb6922c45aecbfdebacc60788ed92d Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 12 Jan 2021 11:31:55 -0800 Subject: [PATCH] Disambiguate Git ref selectors in some Git command line invocations Summary: Ref T13589. See that task for discussion. Test Plan: Executed most commands via "bin/conduit" or in isolation. Maniphest Tasks: T13589 Differential Revision: https://secure.phabricator.com/D21510 --- .../DiffusionBrowseQueryConduitAPIMethod.php | 15 ++++++++------- .../DiffusionExistsQueryConduitAPIMethod.php | 2 +- .../DiffusionHistoryQueryConduitAPIMethod.php | 2 +- ...DiffusionLastModifiedQueryConduitAPIMethod.php | 2 +- ...iffusionMergedCommitsQueryConduitAPIMethod.php | 8 ++++---- .../DiffusionQueryPathsConduitAPIMethod.php | 2 +- .../DiffusionSearchQueryConduitAPIMethod.php | 2 +- .../query/blame/DiffusionGitBlameQuery.php | 2 +- .../filecontent/DiffusionGitFileContentQuery.php | 2 +- .../lowlevel/DiffusionLowLevelParentsQuery.php | 4 ++-- .../query/rawdiff/DiffusionGitRawDiffQuery.php | 6 +++--- 11 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php index 0b6ae19e32..477a89c88b 100644 --- a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php @@ -48,7 +48,7 @@ final class DiffusionBrowseQueryConduitAPIMethod } else { try { list($stdout) = $repository->execxLocalCommand( - 'cat-file -t %s:%s', + 'cat-file -t -- %s:%s', $commit, $path); } catch (CommandException $e) { @@ -62,7 +62,7 @@ final class DiffusionBrowseQueryConduitAPIMethod list($sub_err, $sub_stdout) = $repository->execLocalCommand( 'ls-tree %s -- %s', - $commit, + gitsprintf('%s', $commit), $path); if (!$sub_err) { // If the path failed "cat-file" but "ls-tree" worked, we assume it @@ -86,8 +86,9 @@ final class DiffusionBrowseQueryConduitAPIMethod if (preg_match('/^fatal: Not a valid object name/', $stderr)) { // Grab two logs, since the first one is when the object was deleted. list($stdout) = $repository->execxLocalCommand( - 'log -n2 --format="%%H" %s -- %s', - $commit, + 'log -n2 %s %s -- %s', + '--format=%H', + gitsprintf('%s', $commit), $path); $stdout = trim($stdout); if ($stdout) { @@ -121,8 +122,8 @@ final class DiffusionBrowseQueryConduitAPIMethod } list($stdout) = $repository->execxLocalCommand( - 'ls-tree -z -l %s:%s', - $commit, + 'ls-tree -z -l %s -- %s', + gitsprintf('%s', $commit), $path); $submodules = array(); @@ -207,7 +208,7 @@ final class DiffusionBrowseQueryConduitAPIMethod // the wild. list($err, $contents) = $repository->execLocalCommand( - 'cat-file blob %s:.gitmodules', + 'cat-file blob -- %s:.gitmodules', $commit); if (!$err) { diff --git a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php index 2d4a221171..7a7dbff598 100644 --- a/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionExistsQueryConduitAPIMethod.php @@ -25,7 +25,7 @@ final class DiffusionExistsQueryConduitAPIMethod $repository = $this->getDiffusionRequest()->getRepository(); $commit = $request->getValue('commit'); list($err, $merge_base) = $repository->execLocalCommand( - 'cat-file -t %s', + 'cat-file -t -- %s', $commit); return !$err; } diff --git a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php index ebce21dd6f..223aef6a7d 100644 --- a/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionHistoryQueryConduitAPIMethod.php @@ -64,7 +64,7 @@ final class DiffusionHistoryQueryConduitAPIMethod $offset, $limit, '%H:%P', - $commit_range, + gitsprintf('%s', $commit_range), // Git omits merge commits if the path is provided, even if it is empty. (strlen($path) ? csprintf('%s', $path) : '')); diff --git a/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php index 1135420a6e..a15000bd97 100644 --- a/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionLastModifiedQueryConduitAPIMethod.php @@ -34,7 +34,7 @@ final class DiffusionLastModifiedQueryConduitAPIMethod } list($hash) = $repository->execxLocalCommand( 'log -n1 --format=%%H %s -- %s', - $commit, + gitsprintf('%s', $commit), $path); $results[$path] = trim($hash); } diff --git a/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php index 79587a2e5e..e02580dc73 100644 --- a/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionMergedCommitsQueryConduitAPIMethod.php @@ -35,9 +35,9 @@ final class DiffusionMergedCommitsQueryConduitAPIMethod $limit = $this->getLimit($request); list($parents) = $repository->execxLocalCommand( - 'log -n 1 --format=%s %s', + 'log -n 1 --format=%s %s --', '%P', - $commit); + gitsprintf('%s', $commit)); $parents = preg_split('/\s+/', trim($parents)); if (count($parents) < 2) { @@ -54,8 +54,8 @@ final class DiffusionMergedCommitsQueryConduitAPIMethod // NOTE: "+ 1" accounts for the merge commit itself. $limit + 1, '%H', - $commit, - '^'.$first_parent); + gitsprintf('%s', $commit), + gitsprintf('%s', '^'.$first_parent)); $hashes = explode("\n", trim($logs)); diff --git a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php index 09c07ec28f..98cc006419 100644 --- a/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionQueryPathsConduitAPIMethod.php @@ -45,7 +45,7 @@ final class DiffusionQueryPathsConduitAPIMethod $future = $repository->getLocalCommandFuture( 'ls-tree --name-only -r -z %s -- %s', - $commit, + gitsprintf('%s', $commit), $path); $lines = id(new LinesOfALargeExecFuture($future))->setDelimiter("\0"); diff --git a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php index e2c56bb0f8..ba4c824061 100644 --- a/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionSearchQueryConduitAPIMethod.php @@ -64,7 +64,7 @@ final class DiffusionSearchQueryConduitAPIMethod $future = $repository->getLocalCommandFuture( // NOTE: --perl-regexp is available only with libpcre compiled in. 'grep --extended-regexp --null -n --no-color -f - %s -- %s', - $drequest->getStableCommit(), + gitsprintf('%s', $drequest->getStableCommit()), $path); // NOTE: We're writing the pattern on stdin to avoid issues with UTF8 diff --git a/src/applications/diffusion/query/blame/DiffusionGitBlameQuery.php b/src/applications/diffusion/query/blame/DiffusionGitBlameQuery.php index 0eb15cd018..3525546111 100644 --- a/src/applications/diffusion/query/blame/DiffusionGitBlameQuery.php +++ b/src/applications/diffusion/query/blame/DiffusionGitBlameQuery.php @@ -13,7 +13,7 @@ final class DiffusionGitBlameQuery extends DiffusionBlameQuery { return $repository->getLocalCommandFuture( '--no-pager blame --root -s -l %s -- %s', - $commit, + gitsprintf('%s', $commit), $path); } diff --git a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php index e0fb8bd9ee..8c8e362e60 100644 --- a/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php +++ b/src/applications/diffusion/query/filecontent/DiffusionGitFileContentQuery.php @@ -10,7 +10,7 @@ final class DiffusionGitFileContentQuery extends DiffusionFileContentQuery { $commit = $drequest->getCommit(); return $repository->getLocalCommandFuture( - 'cat-file blob %s:%s', + 'cat-file blob -- %s:%s', $commit, $path); } diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php index a2673b0ce2..e0d9ef14b6 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelParentsQuery.php @@ -37,9 +37,9 @@ final class DiffusionLowLevelParentsQuery $repository = $this->getRepository(); list($stdout) = $repository->execxLocalCommand( - 'log -n 1 --format=%s %s', + 'log -n 1 --format=%s %s --', '%P', - $this->identifier); + gitsprintf('%s', $this->identifier)); return preg_split('/\s+/', trim($stdout)); } diff --git a/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php b/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php index 41d91c00ca..f535724093 100644 --- a/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php +++ b/src/applications/diffusion/query/rawdiff/DiffusionGitRawDiffQuery.php @@ -25,7 +25,7 @@ final class DiffusionGitRawDiffQuery extends DiffusionRawDiffQuery { list($parents) = $repository->execxLocalCommand( 'log -n 1 --format=%s %s --', '%P', - $commit); + gitsprintf('%s', $commit)); if (strlen(trim($parents))) { $against = $commit.'^'; @@ -42,8 +42,8 @@ final class DiffusionGitRawDiffQuery extends DiffusionRawDiffQuery { return $repository->getLocalCommandFuture( 'diff %Ls %s %s -- %s', $options, - $against, - $commit, + gitsprintf('%s', $against), + gitsprintf('%s', $commit), $path); }