Skip to content

Commit 9730f5a

Browse files
author
epriestley
committedNov 30, 2016
Allow custom Sites to have custom 404 controllers
Summary: Currently, custom Sites must match `.*` or similar to handle 404's, since the fallback is always generic. This locks them out of the "redirect to canonicalize to `path/` code", so they currently have a choice between a custom 404 page or automatic correction of `/`. Instead, allow the 404 controller to be constructed explicitly. Sites can now customize 404 by implementing this method and not matching everything. (Sites can still match everything with a catchall rule if they don't want this behavior for some reason, so this should be strictly more powerful than the old behavior.) See next diff for CORGI. Test Plan: - Visited real 404 (like "/asdfafewfq"), missing-slash-404 (like "/maniphest") and real page (like "/maniphest/") URIs on blog, main, and CORGI sites. - Got 404 behavior, redirects, and real pages, respectively. Reviewers: chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16966
1 parent 29a3cd5 commit 9730f5a

File tree

5 files changed

+25
-5
lines changed

5 files changed

+25
-5
lines changed
 

‎src/aphront/AphrontRequest.php

+7
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,13 @@ public function getRequestURI() {
545545
return id(new PhutilURI($path))->setQueryParams($get);
546546
}
547547

548+
public function getAbsoluteRequestURI() {
549+
$uri = $this->getRequestURI();
550+
$uri->setDomain($this->getHost());
551+
$uri->setProtocol($this->isHTTPS() ? 'https' : 'http');
552+
return $uri;
553+
}
554+
548555
public function isDialogFormPost() {
549556
return $this->isFormPost() && $this->getStr('__dialog__');
550557
}

‎src/aphront/configuration/AphrontApplicationConfiguration.php

+10-4
Original file line numberDiff line numberDiff line change
@@ -409,19 +409,25 @@ final private function buildController() {
409409
if (!preg_match('@/$@', $path) && $request->isHTTPGet()) {
410410
$result = $this->routePath($maps, $path.'/');
411411
if ($result) {
412-
$slash_uri = $request->getRequestURI()->setPath($path.'/');
412+
$target_uri = $request->getAbsoluteRequestURI();
413413

414414
// We need to restore URI encoding because the webserver has
415415
// interpreted it. For example, this allows us to redirect a path
416416
// like `/tag/aa%20bb` to `/tag/aa%20bb/`, which may eventually be
417417
// resolved meaningfully by an application.
418-
$slash_uri = phutil_escape_uri($slash_uri);
418+
$target_path = phutil_escape_uri($path.'/');
419+
$target_uri->setPath($target_path);
420+
$target_uri = (string)$target_uri;
419421

420-
$external = strlen($request->getRequestURI()->getDomain());
421-
return $this->buildRedirectController($slash_uri, $external);
422+
return $this->buildRedirectController($target_uri, true);
422423
}
423424
}
424425

426+
$result = $site->new404Controller($request);
427+
if ($result) {
428+
return array($result, array());
429+
}
430+
425431
return $this->build404Controller();
426432
}
427433

‎src/aphront/site/AphrontSite.php

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ abstract public function shouldRequireHTTPS();
99
abstract public function newSiteForRequest(AphrontRequest $request);
1010
abstract public function getRoutingMaps();
1111

12+
public function new404Controller(AphrontRequest $request) {
13+
return null;
14+
}
15+
1216
protected function isHostMatch($host, array $uris) {
1317
foreach ($uris as $uri) {
1418
if (!strlen($uri)) {

‎src/applications/phame/application/PhabricatorPhameApplication.php

-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ private function getLiveRoutes() {
9292
'/' => array(
9393
'' => 'PhameBlogViewController',
9494
'post/(?P<id>\d+)/(?:(?P<slug>[^/]+)/)?' => 'PhamePostViewController',
95-
'.*' => 'PhameBlog404Controller',
9695
),
9796

9897
);

‎src/applications/phame/site/PhameBlogSite.php

+4
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ public function newSiteForRequest(AphrontRequest $request) {
6060
return id(new PhameBlogSite())->setBlog($blog);
6161
}
6262

63+
public function new404Controller(AphrontRequest $request) {
64+
return new PhameBlog404Controller();
65+
}
66+
6367
public function getRoutingMaps() {
6468
$app = PhabricatorApplication::getByClass('PhabricatorPhameApplication');
6569

0 commit comments

Comments
 (0)
Failed to load comments.