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 creation of inline comments with mismatched changesetID / revisionPHID

Summary:
Ref T11092. With Quicksand (or, possibly, some as-yet-unknown non-Quicksand workflow) the client can get stuck with an out-of-date revision PHID.

We then save comments with a `revisionPHID` from one revision and a `changesetID` from a different one.

Detect and prevent this. This stops the workflow immediately when the use first clicks, so it should allow us to detect this issue if it has some other non-Quicksand cause.

Test Plan:
- Opened revision `D123`.
- Pressed `\` to enable the sidebar and Quicksand.
- Clicked a link to revision `D124`.
- Added inlines.

Previously, these could ghost. The exact UI behavior is difficult to describe, but in the database they end up with a `changesetID` for `D124` but the original `revisionPHID` for `D123`, presumably because state is sticking around from the first page.

After this patch, an exception is thrown immediately. Additionally:

- Reloaded to clear quicksand state, added comments fine.
- Disabled sidebar/quicksand, added comments fine.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11092

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

epriestley a34b769b 03e54afc

+24 -3
+24 -3
src/applications/differential/controller/DifferentialInlineCommentEditController.php
··· 23 23 } 24 24 25 25 protected function createComment() { 26 - // Verify revision and changeset correspond to actual objects. 26 + // Verify revision and changeset correspond to actual objects, and are 27 + // connected to one another. 27 28 $changeset_id = $this->getChangesetID(); 29 + $viewer = $this->getViewer(); 28 30 29 31 $revision = $this->loadRevision(); 30 32 31 - if (!id(new DifferentialChangeset())->load($changeset_id)) { 32 - throw new Exception(pht('Invalid changeset ID!')); 33 + $changeset = id(new DifferentialChangesetQuery()) 34 + ->setViewer($viewer) 35 + ->withIDs(array($changeset_id)) 36 + ->executeOne(); 37 + if (!$changeset) { 38 + throw new Exception( 39 + pht( 40 + 'Invalid changeset ID "%s"!', 41 + $changeset_id)); 42 + } 43 + 44 + $diff = $changeset->getDiff(); 45 + if ($diff->getRevisionID() != $revision->getID()) { 46 + throw new Exception( 47 + pht( 48 + 'Changeset ID "%s" is part of diff ID "%s", but that diff '. 49 + 'is attached to reivsion "%s", not revision "%s".', 50 + $changeset_id, 51 + $diff->getID(), 52 + $diff->getRevisionID(), 53 + $revision->getID())); 33 54 } 34 55 35 56 return id(new DifferentialInlineComment())