recaptime.dev / phorge
@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Fix an issue where file queries would throw incorrectly

Summary:
Ref T4589. When you look at a file, we load attached objects in order to run the "you can see this if you can see any attached object" policy check.

However, right now the subquery inherits the "throw on filter" flag from the parent query. This inheritance makes sense in other cases[1], but because this is an "ANY" rule it does not make sense here. In practice, it means that if the file is attached to several objects, and any of them gets filtered, you can not see the file.

Instead, explicitly drop the flag for this subquery.

[1] Sort of. It doesn't produce wrong results in other cases, but now that I think about it might produce a less-tailored error than it could. I'll look into this the next time I'm poking around.

Test Plan:
- Viewed an "All Users" file attached to a private Mock.
- Prior to this patch, I incorrectly received an exception when the Mock was loaded. This is wrong; I should be able to see the file because the policy is "All Users".
- After the patch, I can correctly view the file, just not the associated mock.

{F127074}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: 20after4, aran, epriestley

Maniphest Tasks: T4589

Differential Revision: https://secure.phabricator.com/D8498

epriestley c9fe1624 9181929e

+16 -2
+6
src/applications/files/query/PhabricatorFileQuery.php
··· 137 137 138 138 $objects = array(); 139 139 if ($object_phids) { 140 + // NOTE: We're explicitly turning policy exceptions off, since the rule 141 + // here is "you can see the file if you can see ANY associated object". 142 + // Without this explicit flag, we'll incorrectly throw unless you can 143 + // see ALL associated objects. 144 + 140 145 $objects = id(new PhabricatorObjectQuery()) 141 146 ->setParentQuery($this) 142 147 ->setViewer($this->getViewer()) 143 148 ->withPHIDs($object_phids) 149 + ->setRaisePolicyExceptions(false) 144 150 ->execute(); 145 151 $objects = mpull($objects, null, 'getPHID'); 146 152 }
+10 -2
src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
··· 29 29 abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery { 30 30 31 31 private $viewer; 32 - private $raisePolicyExceptions; 33 32 private $parentQuery; 34 33 private $rawResultLimit; 35 34 private $capabilities; 36 35 private $workspace = array(); 37 36 private $policyFilteredPHIDs = array(); 38 37 private $canUseApplication; 38 + 39 + /** 40 + * Should we continue or throw an exception when a query result is filtered 41 + * by policy rules? 42 + * 43 + * Values are `true` (raise exceptions), `false` (do not raise exceptions) 44 + * and `null` (inherit from parent query, with no exceptions by default). 45 + */ 46 + private $raisePolicyExceptions; 39 47 40 48 41 49 /* -( Query Configuration )------------------------------------------------ */ ··· 186 194 } 187 195 188 196 $parent_query = $this->getParentQuery(); 189 - if ($parent_query) { 197 + if ($parent_query && ($this->raisePolicyExceptions === null)) { 190 198 $this->setRaisePolicyExceptions( 191 199 $parent_query->shouldRaisePolicyExceptions()); 192 200 }