From 03e54afc146c6ed36927f540dd6e40eda681214d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 2 Jun 2016 07:15:39 -0700 Subject: [PATCH] Give Phame blogs an explicit 404 controller Summary: Ref T11076. Ref T9897. Bad links on Phame blogs are currently made worse because we try to prompt you to login on a non-cookie domain. Instead, just 404 in a vanilla way. Do so cleanly on external domains. Test Plan: {F1672399} Reviewers: chad Reviewed By: chad Maniphest Tasks: T9897, T11076 Differential Revision: https://secure.phabricator.com/D16010 --- resources/celerity/map.php | 4 +- src/__phutil_library_map__.php | 4 ++ .../PhabricatorPhameApplication.php | 2 + .../phame/controller/PhameLiveController.php | 33 +++++++++++++- .../blog/PhameBlog404Controller.php | 14 ++++++ .../blog/PhameBlogViewController.php | 6 --- .../phame/site/Phame404Response.php | 43 +++++++++++++++++++ webroot/rsrc/css/application/phame/phame.css | 10 +++++ 8 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 src/applications/phame/controller/blog/PhameBlog404Controller.php create mode 100644 src/applications/phame/site/Phame404Response.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index e462f6d017..c2bee18ab6 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -81,7 +81,7 @@ return array( 'rsrc/css/application/owners/owners-path-editor.css' => '2f00933b', 'rsrc/css/application/paste/paste.css' => '1898e534', 'rsrc/css/application/people/people-profile.css' => '2473d929', - 'rsrc/css/application/phame/phame.css' => '737792d6', + 'rsrc/css/application/phame/phame.css' => '7448a969', 'rsrc/css/application/pholio/pholio-edit.css' => 'b15fec4a', 'rsrc/css/application/pholio/pholio-inline-comments.css' => '8e545e49', 'rsrc/css/application/pholio/pholio.css' => 'ca89d380', @@ -806,7 +806,7 @@ return array( 'phabricator-uiexample-reactor-sendclass' => '1def2711', 'phabricator-uiexample-reactor-sendproperties' => 'b1f0ccee', 'phabricator-zindex-css' => '5b6fcf3f', - 'phame-css' => '737792d6', + 'phame-css' => '7448a969', 'pholio-css' => 'ca89d380', 'pholio-edit-css' => 'b15fec4a', 'pholio-inline-comments-css' => '8e545e49', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index c83772e87e..2533344e43 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3708,7 +3708,9 @@ phutil_register_library_map(array( 'PhabricatorXHProfSample' => 'applications/xhprof/storage/PhabricatorXHProfSample.php', 'PhabricatorXHProfSampleListController' => 'applications/xhprof/controller/PhabricatorXHProfSampleListController.php', 'PhabricatorYoutubeRemarkupRule' => 'infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php', + 'Phame404Response' => 'applications/phame/site/Phame404Response.php', 'PhameBlog' => 'applications/phame/storage/PhameBlog.php', + 'PhameBlog404Controller' => 'applications/phame/controller/blog/PhameBlog404Controller.php', 'PhameBlogArchiveController' => 'applications/phame/controller/blog/PhameBlogArchiveController.php', 'PhameBlogController' => 'applications/phame/controller/blog/PhameBlogController.php', 'PhameBlogCreateCapability' => 'applications/phame/capability/PhameBlogCreateCapability.php', @@ -8508,6 +8510,7 @@ phutil_register_library_map(array( 'PhabricatorXHProfSample' => 'PhabricatorXHProfDAO', 'PhabricatorXHProfSampleListController' => 'PhabricatorXHProfController', 'PhabricatorYoutubeRemarkupRule' => 'PhutilRemarkupRule', + 'Phame404Response' => 'AphrontHTMLResponse', 'PhameBlog' => array( 'PhameDAO', 'PhabricatorPolicyInterface', @@ -8519,6 +8522,7 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', 'PhabricatorConduitResultInterface', ), + 'PhameBlog404Controller' => 'PhameLiveController', 'PhameBlogArchiveController' => 'PhameBlogController', 'PhameBlogController' => 'PhameController', 'PhameBlogCreateCapability' => 'PhabricatorPolicyCapability', diff --git a/src/applications/phame/application/PhabricatorPhameApplication.php b/src/applications/phame/application/PhabricatorPhameApplication.php index 30d1b9cb3d..9138961214 100644 --- a/src/applications/phame/application/PhabricatorPhameApplication.php +++ b/src/applications/phame/application/PhabricatorPhameApplication.php @@ -93,7 +93,9 @@ final class PhabricatorPhameApplication extends PhabricatorApplication { '/' => array( '' => 'PhameBlogViewController', 'post/(?P\d+)/(?:(?P[^/]+)/)?' => 'PhamePostViewController', + '.*' => 'PhameBlog404Controller', ), + ); } diff --git a/src/applications/phame/controller/PhameLiveController.php b/src/applications/phame/controller/PhameLiveController.php index da7fb6da4b..57ac875465 100644 --- a/src/applications/phame/controller/PhameLiveController.php +++ b/src/applications/phame/controller/PhameLiveController.php @@ -70,6 +70,8 @@ abstract class PhameLiveController extends PhameController { $blog = $blog_query->executeOne(); if (!$blog) { + $this->isExternal = $is_external; + $this->isLive = $is_live; return new Aphront404Response(); } @@ -81,6 +83,9 @@ abstract class PhameLiveController extends PhameController { $blog = null; } + $this->isExternal = $is_external; + $this->isLive = $is_live; + if ($post_id) { $post_query = id(new PhamePostQuery()) ->setViewer($viewer) @@ -109,8 +114,6 @@ abstract class PhameLiveController extends PhameController { $post = null; } - $this->isExternal = $is_external; - $this->isLive = $is_live; $this->blog = $blog; $this->post = $post; @@ -188,4 +191,30 @@ abstract class PhameLiveController extends PhameController { return $crumbs; } + public function willSendResponse(AphrontResponse $response) { + if ($this->getIsExternal()) { + if ($response instanceof Aphront404Response) { + $page = $this->newPage() + ->setCrumbs($this->buildApplicationCrumbs()); + + $response = id(new Phame404Response()) + ->setPage($page); + } + } + + return parent::willSendResponse($response); + } + + public function newPage() { + $page = parent::newPage(); + + if ($this->getIsLive()) { + $page + ->setShowChrome(false) + ->setShowFooter(false); + } + + return $page; + } + } diff --git a/src/applications/phame/controller/blog/PhameBlog404Controller.php b/src/applications/phame/controller/blog/PhameBlog404Controller.php new file mode 100644 index 0000000000..994c4d6297 --- /dev/null +++ b/src/applications/phame/controller/blog/PhameBlog404Controller.php @@ -0,0 +1,14 @@ +setupLiveEnvironment(); + if ($response) { + return $response; + } + + return new Aphront404Response(); + } + +} diff --git a/src/applications/phame/controller/blog/PhameBlogViewController.php b/src/applications/phame/controller/blog/PhameBlogViewController.php index ea0822b856..da027d584b 100644 --- a/src/applications/phame/controller/blog/PhameBlogViewController.php +++ b/src/applications/phame/controller/blog/PhameBlogViewController.php @@ -108,12 +108,6 @@ final class PhameBlogViewController extends PhameLiveController { $about, )); - if ($is_live) { - $page - ->setShowChrome(false) - ->setShowFooter(false); - } - return $page; } diff --git a/src/applications/phame/site/Phame404Response.php b/src/applications/phame/site/Phame404Response.php new file mode 100644 index 0000000000..f511a0f9d9 --- /dev/null +++ b/src/applications/phame/site/Phame404Response.php @@ -0,0 +1,43 @@ +page = $page; + return $this; + } + + public function getPage() { + return $this->page; + } + + public function getHTTPResponseCode() { + return 404; + } + + public function buildResponseString() { + require_celerity_resource('phame-css'); + + $not_found = phutil_tag( + 'div', + array( + 'class' => 'phame-404', + ), + array( + phutil_tag('strong', array(), pht('404 Not Found')), + phutil_tag('br'), + pht('Wherever you go, there you are.'), + phutil_tag('br'), + pht('But the page you seek is elsewhere.'), + )); + + $page = $this->getPage() + ->setTitle(pht('404 Not Found')) + ->appendChild($not_found); + + return $page->render(); + } + +} diff --git a/webroot/rsrc/css/application/phame/phame.css b/webroot/rsrc/css/application/phame/phame.css index d4fdeafff0..61c3ce4361 100644 --- a/webroot/rsrc/css/application/phame/phame.css +++ b/webroot/rsrc/css/application/phame/phame.css @@ -243,3 +243,13 @@ left: 50px; position: absolute; } + +.phame-404 { + margin: 48px auto; + padding: 12px 24px; + border-radius: 6px; + min-width: 240px; + width: 50%; + color: {$darkgreytext}; + background: {$greybackground}; +}