HomePhabricator
Diffusion Phabricator 8573d5b0c1a3

Policy - lock down loadCommit() from DiffusionRequest objects
Audited8573d5b0c1a3
Actions

Description

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

Details

Auditors
btrahan
chad
Provenance
btrahanAuthored on
btrahanPushed on Feb 1 2015, 5:33 PM
Reviewer
epriestley
Differential Revision
D11585: Policy - lock down loadCommit() from DiffusionRequest objects
Parents
rP93e6a9b3caad: Allow subscriptions to cost amounts other than one dollar and twenty three cents
Branches
Unknown
Tags
Unknown
Tasks
T7094: Clean up T603

Event Timeline

btrahan committed rP8573d5b0c1a3: Policy - lock down loadCommit() from DiffusionRequest objects (authored by btrahan).Feb 1 2015, 5:33 PM
btrahan added a task: T7094: Clean up T603.
• veblush added a subscriber: • veblush.Feb 2 2015, 12:47 AM

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

[Mon Feb 02 09:34:10.012585 2015] [:error] [pid 1810] [client 192.168.100.29:3949]
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.

• thomasbrixlarsen added a subscriber: • thomasbrixlarsen.Feb 3 2015, 10:18 AM
mormegil added a subscriber: mormegil.Feb 3 2015, 1:38 PM

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).

epriestley mentioned this in T7122: DiffusionPathChangeQuery is fataling with "Call to a member function getTableName() on a non-object".Feb 3 2015, 1:41 PM
epriestley added a subscriber: epriestley.

Filed T7122 to track this, looking at it now.

btrahan accepted this commit.Feb 3 2015, 6:15 PM

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

chad accepted this commit.Feb 3 2015, 6:15 PM

Changes (1)

PathSize
src/
applications/
diffusion/
request/

rP8573d5b0c1a3

View Options

src/applications/diffusion/request/DiffusionRequest.php

Loading...
Phabricator · Built by Phacility