From 36c6eb9663935042c569ac63ecbe72721fdd3316 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 11 Mar 2021 13:07:36 -0800 Subject: [PATCH] Improve routing of "/robots.txt", "/favicon.ico", "/status/", and 404 on custom Sites Summary: Fixes T12919. Fixes T13636. Prior to this change, some well-known resource paths don't route on sites like ResourceSite. - `/robots.txt`: Make it route on ResourceSite and just deny the whole site. - `/favicon.ico`: Make it route on ResourceSite. - `/status/`: Make it route on ResourceSite. - 404: Make it render a 404 on ResourceSite. Test Plan: - Visited all URIs on ResourceSite, got sensible responses. - Visited all URIs on main site. - Visited 404 while logged out, got login page. Maniphest Tasks: T13636, T12919 Differential Revision: https://secure.phabricator.com/D21606 --- src/__phutil_library_map__.php | 6 +++++ .../AphrontApplicationConfiguration.php | 9 +++---- src/aphront/response/Aphront404Response.php | 9 ++++++- src/aphront/site/AphrontSite.php | 2 +- src/aphront/site/PhabricatorPlatformSite.php | 4 +++ .../PhabricatorAuthMainMenuBarExtension.php | 8 +++++- .../controller/Phabricator404Controller.php | 7 +++++- .../PhabricatorPlatform404Controller.php | 10 ++++++++ .../PhabricatorSystemApplication.php | 10 +++++++- .../PhabricatorRobotsController.php | 22 +++++----------- .../PhabricatorRobotsPlatformController.php | 25 +++++++++++++++++++ .../PhabricatorRobotsResourceController.php | 17 +++++++++++++ 12 files changed, 103 insertions(+), 26 deletions(-) create mode 100644 src/applications/base/controller/PhabricatorPlatform404Controller.php create mode 100644 src/applications/system/controller/PhabricatorRobotsPlatformController.php create mode 100644 src/applications/system/controller/PhabricatorRobotsResourceController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 52e5f46bbf..9f02ad2c41 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4253,6 +4253,7 @@ phutil_register_library_map(array( 'PhabricatorPhurlURLViewController' => 'applications/phurl/controller/PhabricatorPhurlURLViewController.php', 'PhabricatorPinnedApplicationsSetting' => 'applications/settings/setting/PhabricatorPinnedApplicationsSetting.php', 'PhabricatorPirateEnglishTranslation' => 'infrastructure/internationalization/translation/PhabricatorPirateEnglishTranslation.php', + 'PhabricatorPlatform404Controller' => 'applications/base/controller/PhabricatorPlatform404Controller.php', 'PhabricatorPlatformSite' => 'aphront/site/PhabricatorPlatformSite.php', 'PhabricatorPointsEditField' => 'applications/transactions/editfield/PhabricatorPointsEditField.php', 'PhabricatorPointsFact' => 'applications/fact/fact/PhabricatorPointsFact.php', @@ -4689,6 +4690,8 @@ phutil_register_library_map(array( 'PhabricatorResetPasswordUserLogType' => 'applications/people/userlog/PhabricatorResetPasswordUserLogType.php', 'PhabricatorResourceSite' => 'aphront/site/PhabricatorResourceSite.php', 'PhabricatorRobotsController' => 'applications/system/controller/PhabricatorRobotsController.php', + 'PhabricatorRobotsPlatformController' => 'applications/system/controller/PhabricatorRobotsPlatformController.php', + 'PhabricatorRobotsResourceController' => 'applications/system/controller/PhabricatorRobotsResourceController.php', 'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php', 'PhabricatorSMSAuthFactor' => 'applications/auth/factor/PhabricatorSMSAuthFactor.php', 'PhabricatorSQLPatchList' => 'infrastructure/storage/patch/PhabricatorSQLPatchList.php', @@ -10914,6 +10917,7 @@ phutil_register_library_map(array( 'PhabricatorPhurlURLViewController' => 'PhabricatorPhurlController', 'PhabricatorPinnedApplicationsSetting' => 'PhabricatorInternalSetting', 'PhabricatorPirateEnglishTranslation' => 'PhutilTranslation', + 'PhabricatorPlatform404Controller' => 'PhabricatorController', 'PhabricatorPlatformSite' => 'PhabricatorSite', 'PhabricatorPointsEditField' => 'PhabricatorEditField', 'PhabricatorPointsFact' => 'PhabricatorFact', @@ -11471,6 +11475,8 @@ phutil_register_library_map(array( 'PhabricatorResetPasswordUserLogType' => 'PhabricatorUserLogType', 'PhabricatorResourceSite' => 'PhabricatorSite', 'PhabricatorRobotsController' => 'PhabricatorController', + 'PhabricatorRobotsPlatformController' => 'PhabricatorRobotsController', + 'PhabricatorRobotsResourceController' => 'PhabricatorRobotsController', 'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine', 'PhabricatorSMSAuthFactor' => 'PhabricatorAuthFactor', 'PhabricatorSQLPatchList' => 'Phobject', diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index 31b3db9a7f..9e22ca7f47 100644 --- a/src/aphront/configuration/AphrontApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontApplicationConfiguration.php @@ -32,10 +32,6 @@ final class AphrontApplicationConfiguration return $request; } - public function build404Controller() { - return array(new Phabricator404Controller(), array()); - } - public function buildRedirectController($uri, $external) { return array( new PhabricatorRedirectController(), @@ -504,7 +500,10 @@ final class AphrontApplicationConfiguration return array($result, array()); } - return $this->build404Controller(); + throw new Exception( + pht( + 'Aphront site ("%s") failed to build a 404 controller.', + get_class($site))); } /** diff --git a/src/aphront/response/Aphront404Response.php b/src/aphront/response/Aphront404Response.php index ea98e9102b..5f2e9f1115 100644 --- a/src/aphront/response/Aphront404Response.php +++ b/src/aphront/response/Aphront404Response.php @@ -10,10 +10,17 @@ final class Aphront404Response extends AphrontHTMLResponse { $request = $this->getRequest(); $viewer = $request->getViewer(); + // See T13636. Note that this response may be served from a Site other than + // the primary PlatformSite. For now, always link to the PlatformSite. + + // (This may not be the best possible place to send users who are currently + // on "real" sites, like the BlogSite.) + $return_uri = PhabricatorEnv::getURI('/'); + $dialog = id(new AphrontDialogView()) ->setViewer($viewer) ->setTitle(pht('404 Not Found')) - ->addCancelButton('/', pht('Return to Charted Waters')) + ->addCancelButton($return_uri, pht('Return to Charted Waters')) ->appendParagraph( pht( 'You arrive at your destination, but there is nothing here.')) diff --git a/src/aphront/site/AphrontSite.php b/src/aphront/site/AphrontSite.php index 9eb053b03b..85bf42fe6c 100644 --- a/src/aphront/site/AphrontSite.php +++ b/src/aphront/site/AphrontSite.php @@ -10,7 +10,7 @@ abstract class AphrontSite extends Phobject { abstract public function getRoutingMaps(); public function new404Controller(AphrontRequest $request) { - return null; + return new Phabricator404Controller(); } protected function isHostMatch($host, array $uris) { diff --git a/src/aphront/site/PhabricatorPlatformSite.php b/src/aphront/site/PhabricatorPlatformSite.php index 0973758bcd..a3193e65d1 100644 --- a/src/aphront/site/PhabricatorPlatformSite.php +++ b/src/aphront/site/PhabricatorPlatformSite.php @@ -50,4 +50,8 @@ final class PhabricatorPlatformSite extends PhabricatorSite { return $maps; } + public function new404Controller(AphrontRequest $request) { + return new PhabricatorPlatform404Controller(); + } + } diff --git a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php index d49a447df6..042a6816dd 100644 --- a/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php +++ b/src/applications/auth/extension/PhabricatorAuthMainMenuBarExtension.php @@ -39,7 +39,13 @@ final class PhabricatorAuthMainMenuBarExtension private function buildLoginMenu() { $controller = $this->getController(); - $uri = new PhutilURI('/auth/start/'); + // See T13636. This button may be rendered by the 404 controller on sites + // other than the primary PlatformSite. Link the button to the primary + // site. + + $uri = '/auth/start/'; + $uri = PhabricatorEnv::getURI($uri); + $uri = new PhutilURI($uri); if ($controller) { $path = $controller->getRequest()->getPath(); $uri->replaceQueryParam('next', $path); diff --git a/src/applications/base/controller/Phabricator404Controller.php b/src/applications/base/controller/Phabricator404Controller.php index 1aa785a040..f181f5badd 100644 --- a/src/applications/base/controller/Phabricator404Controller.php +++ b/src/applications/base/controller/Phabricator404Controller.php @@ -1,6 +1,11 @@ 'PhabricatorStatusController', '/debug/' => 'PhabricatorDebugController', '/favicon.ico' => 'PhabricatorFaviconController', - '/robots.txt' => 'PhabricatorRobotsController', + '/robots.txt' => 'PhabricatorRobotsPlatformController', '/services/' => array( 'encoding/' => 'PhabricatorSystemSelectEncodingController', 'highlight/' => 'PhabricatorSystemSelectHighlightController', @@ -38,4 +38,12 @@ final class PhabricatorSystemApplication extends PhabricatorApplication { ); } + public function getResourceRoutes() { + return array( + '/status/' => 'PhabricatorStatusController', + '/favicon.ico' => 'PhabricatorFaviconController', + '/robots.txt' => 'PhabricatorRobotsResourceController', + ); + } + } diff --git a/src/applications/system/controller/PhabricatorRobotsController.php b/src/applications/system/controller/PhabricatorRobotsController.php index 18be8862a8..c8b73fd947 100644 --- a/src/applications/system/controller/PhabricatorRobotsController.php +++ b/src/applications/system/controller/PhabricatorRobotsController.php @@ -1,26 +1,13 @@ newRobotsRules(); // Add a small crawl delay (number of seconds between requests) for spiders // which respect it. The intent here is to prevent spiders from affecting @@ -36,4 +23,7 @@ final class PhabricatorRobotsController extends PhabricatorController { ->setCacheDurationInSeconds(phutil_units('2 hours in seconds')) ->setCanCDN(true); } + + abstract protected function newRobotsRules(); + } diff --git a/src/applications/system/controller/PhabricatorRobotsPlatformController.php b/src/applications/system/controller/PhabricatorRobotsPlatformController.php new file mode 100644 index 0000000000..5132e94e87 --- /dev/null +++ b/src/applications/system/controller/PhabricatorRobotsPlatformController.php @@ -0,0 +1,25 @@ +