From 9730f5a34fd73fa4f9628fcbdce45a3cb0a4300a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 30 Nov 2016 15:05:40 -0800
Subject: [PATCH] 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
---
 src/aphront/AphrontRequest.php                     |  7 +++++++
 .../AphrontApplicationConfiguration.php            | 14 ++++++++++----
 src/aphront/site/AphrontSite.php                   |  4 ++++
 .../application/PhabricatorPhameApplication.php    |  1 -
 src/applications/phame/site/PhameBlogSite.php      |  4 ++++
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php
index 45b383c839..6045c0d728 100644
--- a/src/aphront/AphrontRequest.php
+++ b/src/aphront/AphrontRequest.php
@@ -545,6 +545,13 @@ final class AphrontRequest extends Phobject {
     return id(new PhutilURI($path))->setQueryParams($get);
   }
 
+  public function getAbsoluteRequestURI() {
+    $uri = $this->getRequestURI();
+    $uri->setDomain($this->getHost());
+    $uri->setProtocol($this->isHTTPS() ? 'https' : 'http');
+    return $uri;
+  }
+
   public function isDialogFormPost() {
     return $this->isFormPost() && $this->getStr('__dialog__');
   }
diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php
index c39dd2033a..bcbc325b87 100644
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@@ -409,19 +409,25 @@ abstract class AphrontApplicationConfiguration extends Phobject {
     if (!preg_match('@/$@', $path) && $request->isHTTPGet()) {
       $result = $this->routePath($maps, $path.'/');
       if ($result) {
-        $slash_uri = $request->getRequestURI()->setPath($path.'/');
+        $target_uri = $request->getAbsoluteRequestURI();
 
         // We need to restore URI encoding because the webserver has
         // interpreted it. For example, this allows us to redirect a path
         // like `/tag/aa%20bb` to `/tag/aa%20bb/`, which may eventually be
         // resolved meaningfully by an application.
-        $slash_uri = phutil_escape_uri($slash_uri);
+        $target_path = phutil_escape_uri($path.'/');
+        $target_uri->setPath($target_path);
+        $target_uri = (string)$target_uri;
 
-        $external = strlen($request->getRequestURI()->getDomain());
-        return $this->buildRedirectController($slash_uri, $external);
+        return $this->buildRedirectController($target_uri, true);
       }
     }
 
+    $result = $site->new404Controller($request);
+    if ($result) {
+      return array($result, array());
+    }
+
     return $this->build404Controller();
   }
 
diff --git a/src/aphront/site/AphrontSite.php b/src/aphront/site/AphrontSite.php
index b4d4fed0d1..9eb053b03b 100644
--- a/src/aphront/site/AphrontSite.php
+++ b/src/aphront/site/AphrontSite.php
@@ -9,6 +9,10 @@ abstract class AphrontSite extends Phobject {
   abstract public function newSiteForRequest(AphrontRequest $request);
   abstract public function getRoutingMaps();
 
+  public function new404Controller(AphrontRequest $request) {
+    return null;
+  }
+
   protected function isHostMatch($host, array $uris) {
     foreach ($uris as $uri) {
       if (!strlen($uri)) {
diff --git a/src/applications/phame/application/PhabricatorPhameApplication.php b/src/applications/phame/application/PhabricatorPhameApplication.php
index 873219886b..8b4d991591 100644
--- a/src/applications/phame/application/PhabricatorPhameApplication.php
+++ b/src/applications/phame/application/PhabricatorPhameApplication.php
@@ -92,7 +92,6 @@ final class PhabricatorPhameApplication extends PhabricatorApplication {
       '/' => array(
         '' => 'PhameBlogViewController',
         'post/(?P<id>\d+)/(?:(?P<slug>[^/]+)/)?' => 'PhamePostViewController',
-        '.*' => 'PhameBlog404Controller',
       ),
 
     );
diff --git a/src/applications/phame/site/PhameBlogSite.php b/src/applications/phame/site/PhameBlogSite.php
index 20bdfd5230..f45aca0266 100644
--- a/src/applications/phame/site/PhameBlogSite.php
+++ b/src/applications/phame/site/PhameBlogSite.php
@@ -60,6 +60,10 @@ final class PhameBlogSite extends PhameSite {
     return id(new PhameBlogSite())->setBlog($blog);
   }
 
+  public function new404Controller(AphrontRequest $request) {
+    return new PhameBlog404Controller();
+  }
+
   public function getRoutingMaps() {
     $app = PhabricatorApplication::getByClass('PhabricatorPhameApplication');
 
-- 
2.51.2