Policy - lock down loadCommit() from DiffusionRequest objects


Policy - lock down loadCommit() from DiffusionRequest objects

Summary: Ref T7094. The class DiffusionRequest has other public methods which use getUser() in an unguarded way. Code inspection of the call sites for loadCommit() also leads me to believe the $user is properly set.

Test Plan: clicked around diffusion a bunch and everything seemed to work okay. (happy to test any particular esoteric endpoints that come to mind)

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7094

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


Event Timeline

I found that this commit caused a following error on my server.

[Mon Feb 02 09:34:10.012585 2015] [:error] [pid 1810] [client]
PHP Fatal error:  Call to a member function getTableName() on a non-object in ~/Phabricator/phabricator/src/applications/diffusion/query/pathchange/DiffusionPathChangeQuery.php on line 66

After rollback, error is gone. I don't know why but please check it out.
Thanks in advance.

chad raised a concern with this commit.Feb 2 2015, 4:18 AM
chad added a subscriber: chad.

Placing on our radar.

I have the same problem, as I have commented at GitHub: It seems to me DiffusionCommitQuery::buildWhereClause expects/supports only Git-like revision hashes, and does not support traditional revision numbers (for Subversion).

Resolved via @T7122. Pretty sure @chad will need to accept this to clear it out from needs attention queue.