From d6fd365704072d8b5365de0c13efec22efd8b501 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 27 Jan 2021 16:31:08 -0800 Subject: [PATCH] Correct Diffusion browse behavior when visiting a path URI with no trailing slash Summary: See PHI1983. Ref T13599. Ref T13589. Currently, if you browse to a path browse URI in Diffusion without a trailing slash (`/browse/master/src`), you get a nonsensical view (the directory as a single item). Be more precise in how "git ls-tree" arguments are constructed. Test Plan: Visited files and directories in the browse view, with and without trailing slashes. Saw improved behavior for directories with no trailing slash and reasonable behavior in all other cases. Maniphest Tasks: T13599, T13589 Differential Revision: https://secure.phabricator.com/D21528 --- .../DiffusionBrowseQueryConduitAPIMethod.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php index d3fe3b922e..deda3d43d7 100644 --- a/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionBrowseQueryConduitAPIMethod.php @@ -37,7 +37,7 @@ final class DiffusionBrowseQueryConduitAPIMethod $repository = $drequest->getRepository(); $path = $request->getValue('path'); - if (!strlen($path)) { + if (!strlen($path) || $path === '/') { $path = null; } @@ -49,12 +49,13 @@ final class DiffusionBrowseQueryConduitAPIMethod if ($path === null) { // Fast path to improve the performance of the repository view; we know // the root is always a tree at any commit and always exists. - $stdout = 'tree'; + $path_type = 'tree'; } else { try { list($stdout) = $repository->execxLocalCommand( 'cat-file -t -- %s', sprintf('%s:%s', $commit, $path)); + $path_type = trim($stdout); } catch (CommandException $e) { // The "cat-file" command may fail if the path legitimately does not // exist, but it may also fail if the path is a submodule. This can @@ -114,7 +115,7 @@ final class DiffusionBrowseQueryConduitAPIMethod } } - if (trim($stdout) == 'blob') { + if ($path_type === 'blob') { $result->setReasonForEmptyResultSet( DiffusionBrowseResultSet::REASON_IS_FILE); return $result; @@ -130,6 +131,12 @@ final class DiffusionBrowseQueryConduitAPIMethod 'ls-tree -z -l %s --', gitsprintf('%s', $commit)); } else { + if ($path_type === 'tree') { + $path = rtrim($path, '/').'/'; + } else { + $path = rtrim($path, '/'); + } + list($stdout) = $repository->execxLocalCommand( 'ls-tree -z -l %s -- %s', gitsprintf('%s', $commit),