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.

Prevent Files from requiring infinite policy checks

Summary:
Fixes T6726. Currently, a file may be attached to itself (or to other files, ultimately forming a loop). In this case, we currently run around the loop forever trying to load all the files.

Instead, decline to load objects if we're inside a query which is already loading them. This produces the right policy result //and// completes in finite time.

Test Plan:
- Looped two files by writing `{F123}` and `{F124}` on the other files, respectively.
- Loaded `F123`.
- Saw long hang; used `debug.time-limit` to see huge stack trace instead.
- Wrote patch.
- `F123` now loads correctly.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T6726

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

epriestley 7556a702 a238f6a7

+46
+12
src/applications/phid/query/PhabricatorObjectQuery.php
··· 104 104 } 105 105 106 106 private function loadObjectsByPHID(array $types, array $phids) { 107 + // Don't try to load PHIDs which are already "in flight"; this prevents us 108 + // from recursing indefinitely if policy checks or edges form a loop. We 109 + // will decline to load the corresponding objects. 110 + $in_flight = $this->getPHIDsInFlight(); 111 + foreach ($phids as $key => $phid) { 112 + if (isset($in_flight[$phid])) { 113 + unset($phids[$key]); 114 + } 115 + } 116 + 107 117 $results = array(); 108 118 109 119 $workspace = $this->getObjectsFromWorkspace($phids); ··· 118 128 if (!$phids) { 119 129 return $results; 120 130 } 131 + 132 + $this->putPHIDsInFlight($phids); 121 133 122 134 $groups = array(); 123 135 foreach ($phids as $phid) {
+34
src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
··· 33 33 private $rawResultLimit; 34 34 private $capabilities; 35 35 private $workspace = array(); 36 + private $inFlightPHIDs = array(); 36 37 private $policyFilteredPHIDs = array(); 37 38 private $canUseApplication; 38 39 ··· 465 466 } 466 467 467 468 return $map; 469 + } 470 + 471 + 472 + /** 473 + * Mark PHIDs as in flight. 474 + * 475 + * PHIDs which are "in flight" are actively being queried for. Using this 476 + * list can prevent infinite query loops by aborting queries which cycle. 477 + * 478 + * @param list<phid> List of PHIDs which are now in flight. 479 + * @return this 480 + */ 481 + public function putPHIDsInFlight(array $phids) { 482 + foreach ($phids as $phid) { 483 + $this->inFlightPHIDs[$phid] = $phid; 484 + } 485 + return $this; 486 + } 487 + 488 + 489 + /** 490 + * Get PHIDs which are currently in flight. 491 + * 492 + * PHIDs which are "in flight" are actively being queried for. 493 + * 494 + * @return map<phid, phid> PHIDs currently in flight. 495 + */ 496 + public function getPHIDsInFlight() { 497 + $results = $this->inFlightPHIDs; 498 + if ($this->getParentQuery()) { 499 + $results += $this->getParentQuery()->getPHIDsInFlight(); 500 + } 501 + return $results; 468 502 } 469 503 470 504