From 5240cffd9c20c4f759b4ab2327ec4f6fbe2063ab Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 30 Nov 2017 12:33:02 -0800 Subject: [PATCH] Fix an issue where Diffusion could fatal if the default branch was deleted Summary: See PHI234. In T12931 we improved the behavior of Diffusion when a repository's default branch is set to a branch that does not exist, but in T11823 the way refcursors work changed, and we can now get a cursor (just with no positions) back for a deleted branch. When we did, we didn't handle things gracefully. Test Plan: - Set default branch to a deleted branch, saw nice error instead of fatal. - Set default branch to a nonexistent branch which never existed, saw nice error. - Set default branch to existing "master", saw repository normally. Reviewers: amckinley Reviewed By: amckinley Differential Revision: https://secure.phabricator.com/D18811 --- .../controller/DiffusionRepositoryController.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/applications/diffusion/controller/DiffusionRepositoryController.php b/src/applications/diffusion/controller/DiffusionRepositoryController.php index f64b72e1f8..35fce7f03d 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryController.php @@ -46,8 +46,20 @@ final class DiffusionRepositoryController extends DiffusionController { ->withRepositoryPHIDs(array($repository->getPHID())) ->withRefTypes(array(PhabricatorRepositoryRefCursor::TYPE_BRANCH)) ->withRefNames(array($drequest->getBranch())) + ->needPositions(true) ->execute(); - if ($ref_cursors) { + + // It's possible that this branch previously existed, but has been + // deleted. Make sure we have valid cursor positions, not just cursors. + $any_positions = false; + foreach ($ref_cursors as $ref_cursor) { + if ($ref_cursor->getPositions()) { + $any_positions = true; + break; + } + } + + if ($any_positions) { // This is a valid branch, so we necessarily have some content. $page_has_content = true; } else {