1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-20 20:40:56 +01:00

Diffusion - return 404 errors for bad URIs

Summary: Fixes T5646. Makes diffusion a much better user experience. Users now see a 404 exception page when they have a bad URI. Previously, they saw a developer-facing raw exception.

Test Plan: played around in diffusion a bunch. most of these changes were fairly mechanical at the end of the day.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T5646

Differential Revision: https://secure.phabricator.com/D11299
This commit is contained in:
Bob Trahan 2015-01-09 13:29:08 -08:00
parent 11a20079ef
commit a823654be0
45 changed files with 120 additions and 185 deletions

View file

@ -6,9 +6,8 @@ final class DiffusionBranchTableController extends DiffusionController {
return true;
}
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->getDiffusionRequest();
$request = $this->getRequest();
$viewer = $request->getUser();
$repository = $drequest->getRepository();

View file

@ -14,7 +14,7 @@ final class DiffusionBrowseDirectoryController
return $this->browseQueryResults;
}
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->diffusionRequest;
$results = $this->getBrowseQueryResults();
@ -35,7 +35,7 @@ final class DiffusionBrowseDirectoryController
$empty_result = new DiffusionEmptyResultView();
$empty_result->setDiffusionRequest($drequest);
$empty_result->setDiffusionBrowseResultSet($results);
$empty_result->setView($this->getRequest()->getStr('view'));
$empty_result->setView($request->getStr('view'));
$content[] = $empty_result;
} else {
$phids = array();
@ -55,7 +55,7 @@ final class DiffusionBrowseDirectoryController
$browse_table->setDiffusionRequest($drequest);
$browse_table->setHandles($handles);
$browse_table->setPaths($results->getPaths());
$browse_table->setUser($this->getRequest()->getUser());
$browse_table->setUser($request->getUser());
$browse_panel = new AphrontPanelView();
$browse_panel->appendChild($browse_table);

View file

@ -6,8 +6,7 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
private $lintMessages;
private $coverage;
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->getDiffusionRequest();
$viewer = $request->getUser();

View file

@ -2,9 +2,8 @@
final class DiffusionBrowseMainController extends DiffusionBrowseController {
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->diffusionRequest;
$request = $this->getRequest();
// Figure out if we're browsing a directory, a file, or a search result
// list. Then delegate to the appropriate controller.

View file

@ -2,7 +2,7 @@
final class DiffusionBrowseSearchController extends DiffusionBrowseController {
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->diffusionRequest;
$actions = $this->buildActionView($drequest);

View file

@ -6,9 +6,9 @@ final class DiffusionChangeController extends DiffusionController {
return true;
}
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->diffusionRequest;
$viewer = $this->getRequest()->getUser();
$viewer = $request->getUser();
$content = array();
@ -62,7 +62,7 @@ final class DiffusionChangeController extends DiffusionController {
$changeset_view->setRenderURI('/diffusion/'.$callsign.'/diff/');
$changeset_view->setWhitespace(
DifferentialChangesetParser::WHITESPACE_SHOW_ALL);
$changeset_view->setUser($this->getRequest()->getUser());
$changeset_view->setUser($viewer);
// TODO: This is pretty awkward, unify the CSS between Diffusion and
// Differential better.

View file

@ -6,20 +6,15 @@ final class DiffusionCommitBranchesController extends DiffusionController {
return true;
}
public function willProcessRequest(array $data) {
$data['user'] = $this->getRequest()->getUser();
$this->diffusionRequest = DiffusionRequest::newFromDictionary($data);
}
public function processRequest() {
$request = $this->getDiffusionRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->getDiffusionRequest();
$branches = array();
try {
$branches = $this->callConduitWithDiffusionRequest(
'diffusion.branchquery',
array(
'contains' => $request->getCommit(),
'contains' => $drequest->getCommit(),
));
} catch (ConduitException $ex) {
if ($ex->getMessage() != 'ERR-UNSUPPORTED-VCS') {
@ -34,7 +29,7 @@ final class DiffusionCommitBranchesController extends DiffusionController {
$branch_links[] = phutil_tag(
'a',
array(
'href' => $request->generateURI(
'href' => $drequest->generateURI(
array(
'action' => 'browse',
'branch' => $branch->getShortName(),

View file

@ -11,19 +11,18 @@ final class DiffusionCommitController extends DiffusionController {
return true;
}
public function willProcessRequest(array $data) {
// This controller doesn't use blob/path stuff, just pass the dictionary
// in directly instead of using the AphrontRequest parsing mechanism.
$data['user'] = $this->getRequest()->getUser();
$drequest = DiffusionRequest::newFromDictionary($data);
$this->diffusionRequest = $drequest;
protected function shouldLoadDiffusionRequest() {
return false;
}
public function processRequest() {
$drequest = $this->getDiffusionRequest();
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
// This controller doesn't use blob/path stuff, just pass the dictionary
// in directly instead of using the AphrontRequest parsing mechanism.
$data = $request->getURIMap();
$data['user'] = $user;
$drequest = DiffusionRequest::newFromDictionary($data);
$this->diffusionRequest = $drequest;
if ($request->getStr('diff')) {
return $this->buildRawDiffResponse($drequest);

View file

@ -2,13 +2,7 @@
final class DiffusionCommitEditController extends DiffusionController {
public function willProcessRequest(array $data) {
$data['user'] = $this->getRequest()->getUser();
$this->diffusionRequest = DiffusionRequest::newFromDictionary($data);
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
$drequest = $this->getDiffusionRequest();
$callsign = $drequest->getRepository()->getCallsign();

View file

@ -6,13 +6,8 @@ final class DiffusionCommitTagsController extends DiffusionController {
return true;
}
public function willProcessRequest(array $data) {
$data['user'] = $this->getRequest()->getUser();
$this->diffusionRequest = DiffusionRequest::newFromDictionary($data);
}
public function processRequest() {
$request = $this->getDiffusionRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->getDiffusionRequest();
$tag_limit = 10;
$tags = array();
@ -21,7 +16,7 @@ final class DiffusionCommitTagsController extends DiffusionController {
$this->callConduitWithDiffusionRequest(
'diffusion.tagsquery',
array(
'commit' => $request->getCommit(),
'commit' => $drequest->getCommit(),
'limit' => $tag_limit + 1,
)));
} catch (ConduitException $ex) {
@ -38,7 +33,7 @@ final class DiffusionCommitTagsController extends DiffusionController {
$tag_links[] = phutil_tag(
'a',
array(
'href' => $request->generateURI(
'href' => $drequest->generateURI(
array(
'action' => 'browse',
'commit' => $tag->getName(),
@ -51,7 +46,7 @@ final class DiffusionCommitTagsController extends DiffusionController {
$tag_links[] = phutil_tag(
'a',
array(
'href' => $request->generateURI(
'href' => $drequest->generateURI(
array(
'action' => 'tags',
)),

View file

@ -31,15 +31,28 @@ abstract class DiffusionController extends PhabricatorController {
return parent::willBeginExecution();
}
public function willProcessRequest(array $data) {
if (isset($data['callsign'])) {
protected function shouldLoadDiffusionRequest() {
return true;
}
final public function handleRequest(AphrontRequest $request) {
if ($request->getURIData('callsign') &&
$this->shouldLoadDiffusionRequest()) {
try {
$drequest = DiffusionRequest::newFromAphrontRequestDictionary(
$data,
$this->getRequest());
$request->getURIMap(),
$request);
} catch (Exception $ex) {
return id(new Aphront404Response())
->setRequest($request);
}
$this->setDiffusionRequest($drequest);
}
return $this->processDiffusionRequest($request);
}
abstract protected function processDiffusionRequest(AphrontRequest $request);
public function buildCrumbs(array $spec = array()) {
$crumbs = $this->buildApplicationCrumbs();
$crumb_list = $this->buildCrumbList($spec);

View file

@ -6,20 +6,26 @@ final class DiffusionDiffController extends DiffusionController {
return true;
}
public function willProcessRequest(array $data) {
protected function shouldLoadDiffusionRequest() {
return false;
}
protected function processDiffusionRequest(AphrontRequest $request) {
$data = $request->getURIMap();
$data = $data + array(
'dblob' => $this->getRequest()->getStr('ref'),
);
$drequest = DiffusionRequest::newFromAphrontRequestDictionary(
$data,
$this->getRequest());
try {
$drequest = DiffusionRequest::newFromAphrontRequestDictionary(
$data,
$request);
} catch (Exception $ex) {
return id(new Aphront404Response())
->setRequest($request);
}
$this->setDiffusionRequest($drequest);
$this->diffusionRequest = $drequest;
}
public function processRequest() {
$drequest = $this->getDiffusionRequest();
$request = $this->getRequest();
$user = $request->getUser();
if (!$request->isAjax()) {

View file

@ -2,16 +2,15 @@
final class DiffusionExternalController extends DiffusionController {
public function willProcessRequest(array $data) {
// Don't build a DiffusionRequest.
}
public function shouldAllowPublic() {
return true;
}
public function processRequest() {
$request = $this->getRequest();
protected function shouldLoadDiffusionRequest() {
return false;
}
protected function processDiffusionRequest(AphrontRequest $request) {
$uri = $request->getStr('uri');
$id = $request->getStr('id');

View file

@ -6,9 +6,8 @@ final class DiffusionHistoryController extends DiffusionController {
return true;
}
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->diffusionRequest;
$request = $this->getRequest();
$viewer = $request->getUser();
$repository = $drequest->getRepository();

View file

@ -6,9 +6,8 @@ final class DiffusionLastModifiedController extends DiffusionController {
return true;
}
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->getDiffusionRequest();
$request = $this->getRequest();
$viewer = $request->getUser();
$paths = $request->getStr('paths');

View file

@ -6,9 +6,8 @@ final class DiffusionLintController extends DiffusionController {
return true;
}
public function processRequest() {
$request = $this->getRequest();
$user = $this->getRequest()->getUser();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
$drequest = $this->diffusionRequest;
if ($request->getStr('lint') !== null) {

View file

@ -2,9 +2,9 @@
final class DiffusionLintDetailsController extends DiffusionController {
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$limit = 500;
$offset = $this->getRequest()->getInt('offset', 0);
$offset = $request->getInt('offset', 0);
$drequest = $this->getDiffusionRequest();
$branch = $drequest->loadBranch();
@ -70,7 +70,7 @@ final class DiffusionLintDetailsController extends DiffusionController {
->setPageSize($limit)
->setOffset($offset)
->setHasMorePages(count($messages) >= $limit)
->setURI($this->getRequest()->getRequestURI(), 'offset');
->setURI($request->getRequestURI(), 'offset');
$content[] = id(new AphrontPanelView())
->setNoBackground(true)

View file

@ -3,22 +3,14 @@
final class DiffusionMirrorDeleteController
extends DiffusionController {
private $id;
public function willProcessRequest(array $data) {
$this->id = $data['id'];
parent::willProcessRequest($data);
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();
$mirror = id(new PhabricatorRepositoryMirrorQuery())
->setViewer($viewer)
->withIDs(array($this->id))
->withIDs(array($request->getURIData('id')))
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,

View file

@ -3,15 +3,7 @@
final class DiffusionMirrorEditController
extends DiffusionController {
private $id;
public function willProcessRequest(array $data) {
$this->id = idx($data, 'id');
parent::willProcessRequest($data);
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();
@ -21,10 +13,10 @@ final class DiffusionMirrorEditController
$repository,
PhabricatorPolicyCapability::CAN_EDIT);
if ($this->id) {
if ($request->getURIData('id')) {
$mirror = id(new PhabricatorRepositoryMirrorQuery())
->setViewer($viewer)
->withIDs(array($this->id))
->withIDs(array($request->getURIData('id')))
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,

View file

@ -2,12 +2,11 @@
final class DiffusionPathCompleteController extends DiffusionController {
public function willProcessRequest(array $data) {
// Don't build a DiffusionRequest.
protected function shouldLoadDiffusionRequest() {
return false;
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$repository_phid = $request->getStr('repositoryPHID');
$repository = id(new PhabricatorRepositoryQuery())

View file

@ -2,7 +2,7 @@
final class DiffusionPathTreeController extends DiffusionController {
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->getDiffusionRequest();
if (!$drequest->getRepository()->canUsePathTree()) {

View file

@ -2,12 +2,11 @@
final class DiffusionPathValidateController extends DiffusionController {
public function willProcessRequest(array $data) {
// Don't build a DiffusionRequest.
protected function shouldLoadDiffusionRequest() {
return false;
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$repository_phid = $request->getStr('repositoryPHID');
$repository = id(new PhabricatorRepositoryQuery())

View file

@ -3,23 +3,16 @@
final class DiffusionPushEventViewController
extends DiffusionPushLogController {
private $id;
public function shouldAllowPublic() {
return true;
}
public function willProcessRequest(array $data) {
$this->id = idx($data, 'id');
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$event = id(new PhabricatorRepositoryPushEventQuery())
->setViewer($viewer)
->withIDs(array($this->id))
->withIDs(array($request->getURIData('id')))
->needLogs(true)
->executeOne();
if (!$event) {

View file

@ -1,3 +1,9 @@
<?php
abstract class DiffusionPushLogController extends DiffusionController {}
abstract class DiffusionPushLogController extends DiffusionController {
protected function shouldLoadDiffusionRequest() {
return false;
}
}

View file

@ -2,20 +2,14 @@
final class DiffusionPushLogListController extends DiffusionPushLogController {
private $queryKey;
public function shouldAllowPublic() {
return true;
}
public function willProcessRequest(array $data) {
$this->queryKey = idx($data, 'queryKey');
}
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$request = $this->getRequest();
$controller = id(new PhabricatorApplicationSearchController())
->setQueryKey($this->queryKey)
->setQueryKey($request->getURIData('queryKey'))
->setSearchEngine(new PhabricatorRepositoryPushLogSearchEngine())
->setNavigation($this->buildSideNavView());

View file

@ -6,8 +6,7 @@ final class DiffusionRepositoryController extends DiffusionController {
return true;
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->getDiffusionRequest();

View file

@ -6,14 +6,9 @@ final class DiffusionRepositoryCreateController
private $edit;
private $repository;
public function willProcessRequest(array $data) {
parent::willProcessRequest($data);
$this->edit = $data['edit'];
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$this->edit = $request->getURIData('edit');
// NOTE: We can end up here via either "Create Repository", or via
// "Import Repository", or via "Edit Remote", or via "Edit Policies". In

View file

@ -2,7 +2,7 @@
final class DiffusionRepositoryDefaultController extends DiffusionController {
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
// NOTE: This controller is just here to make sure we call
// willBeginExecution() on any /diffusion/X/ URI, so we can intercept
// `git`, `hg` and `svn` HTTP protocol requests.

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditActionsController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditActivateController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditBasicController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,7 +3,7 @@
final class DiffusionRepositoryEditBranchesController
extends DiffusionRepositoryEditController {
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$request = $this->getRequest();
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditDangerousController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditDeleteController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditEncodingController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -5,16 +5,11 @@ final class DiffusionRepositoryEditHostingController
private $serve;
public function willProcessRequest(array $data) {
parent::willProcessRequest($data);
$this->serve = idx($data, 'serve');
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();
$this->serve = $request->getURIData('serve');
$repository = id(new PhabricatorRepositoryQuery())
->setViewer($user)

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditMainController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditStorageController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditSubversionController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -3,8 +3,7 @@
final class DiffusionRepositoryEditUpdateController
extends DiffusionRepositoryEditController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$drequest = $this->diffusionRequest;
$repository = $drequest->getRepository();

View file

@ -2,20 +2,13 @@
final class DiffusionRepositoryListController extends DiffusionController {
private $queryKey;
public function shouldAllowPublic() {
return true;
}
public function willProcessRequest(array $data) {
$this->queryKey = idx($data, 'queryKey');
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$controller = id(new PhabricatorApplicationSearchController())
->setQueryKey($this->queryKey)
->setQueryKey($request->getURIData('queryKey'))
->setSearchEngine(new PhabricatorRepositorySearchEngine())
->setNavigation($this->buildSideNavView());

View file

@ -2,8 +2,7 @@
final class DiffusionRepositoryNewController extends DiffusionController {
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$viewer = $request->getUser();
$this->requireApplicationCapability(

View file

@ -55,8 +55,7 @@ final class DiffusionServeController extends DiffusionController {
return $matches['callsign'];
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$callsign = self::getCallsign($request);
// If authentication credentials have been provided, try to find a user

View file

@ -4,13 +4,9 @@ final class DiffusionSymbolController extends DiffusionController {
private $name;
public function willProcessRequest(array $data) {
$this->name = $data['name'];
}
public function processRequest() {
$request = $this->getRequest();
protected function processDiffusionRequest(AphrontRequest $request) {
$user = $request->getUser();
$this->name = $request->getURIData('name');
$query = new DiffusionSymbolQuery();
$query->setName($this->name);

View file

@ -6,9 +6,8 @@ final class DiffusionTagListController extends DiffusionController {
return true;
}
public function processRequest() {
protected function processDiffusionRequest(AphrontRequest $request) {
$drequest = $this->getDiffusionRequest();
$request = $this->getRequest();
$viewer = $request->getUser();
$repository = $drequest->getRepository();