From 2fbc65e3964991ed4b358d19efcd7b8fd5fa1a9d Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sun, 14 Jun 2015 15:35:32 -0700
Subject: [PATCH] Call didRejectResult() in DiffusionCommitQuery properly

Summary:
Ref T4345. This error is per object-type in the query implementations, not a mail/permissions issue.

Without `didRejectResult()`, we can't distinguish between "restricted" and "unknown" for objects filtered by `willFilterPage()`.

  - Call `didRejectResult()` on commits.
  - Make `didRejectResult()` handle both existing policy exceptions and filtering.
  - Recover from partial objects (like commits) which are missing attached data required to figure out policies.

Test Plan: Saw "Restricted Diffusion Commit" instead of "Unknown Object (Diffusion Commit)" when viewing nonvisible commit handle in Maniphest.

Reviewers: btrahan, joshuaspence

Reviewed By: joshuaspence

Subscribers: epriestley

Maniphest Tasks: T4345

Differential Revision: https://secure.phabricator.com/D13289
---
 .../diffusion/query/DiffusionCommitQuery.php   |  1 +
 .../policy/PhabricatorPolicyAwareQuery.php     | 18 +++++++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/applications/diffusion/query/DiffusionCommitQuery.php b/src/applications/diffusion/query/DiffusionCommitQuery.php
index 93efb1df12..75913ba37e 100644
--- a/src/applications/diffusion/query/DiffusionCommitQuery.php
+++ b/src/applications/diffusion/query/DiffusionCommitQuery.php
@@ -193,6 +193,7 @@ final class DiffusionCommitQuery
       if ($repo) {
         $commit->attachRepository($repo);
       } else {
+        $this->didRejectResult($commit);
         unset($commits[$key]);
         continue;
       }
diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
index 1d8f10b916..e7b9b35628 100644
--- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
@@ -338,9 +338,25 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
   }
 
   protected function didRejectResult(PhabricatorPolicyInterface $object) {
+    // Some objects (like commits) may be rejected because related objects
+    // (like repositories) can not be loaded. In some cases, we may need these
+    // related objects to determine the object policy, so it's expected that
+    // we may occasionally be unable to determine the policy.
+
+    try {
+      $policy = $object->getPolicy(PhabricatorPolicyCapability::CAN_VIEW);
+    } catch (Exception $ex) {
+      $policy = null;
+    }
+
+    // Mark this object as filtered so handles can render "Restricted" instead
+    // of "Unknown".
+    $phid = $object->getPHID();
+    $this->addPolicyFilteredPHIDs(array($phid => $phid));
+
     $this->getPolicyFilter()->rejectObject(
       $object,
-      $object->getPolicy(PhabricatorPolicyCapability::CAN_VIEW),
+      $policy,
       PhabricatorPolicyCapability::CAN_VIEW);
   }