From 1fc60a9a6e70ca9e48599ad8c4e7309f6747f46a Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 3 Sep 2015 10:04:42 -0700 Subject: [PATCH] Modularize Aphront exception handling Summary: Ref T1806. Ref T7173. Depends on D14047. Currently, all exception handling is in this big messy clump in `AphrontDefaultApplicationConfiguration`. Split it out into modular classes. This will let a future change add new classes in the Phacility cluster which intercept particular exceptions we care about and replaces the default, generic responses with more useful, tailored responses. Test Plan: {F777391} - Hit a Conduit error (made a method throw). - Hit an Ajax error (made comment preview throw). - Hit a high security error (tried to edit TOTP). - Hit a rate limiting error (added a bunch of email addresses). - Hit a policy error (tried to look at something with no permission). - Hit an arbitrary exception (made a randomc ontroller throw). Reviewers: chad Reviewed By: chad Maniphest Tasks: T1806, T7173 Differential Revision: https://secure.phabricator.com/D14049 --- src/__phutil_library_map__.php | 18 ++ .../AphrontApplicationConfiguration.php | 64 +++++- ...AphrontDefaultApplicationConfiguration.php | 213 ------------------ .../AphrontRequestExceptionHandler.php | 36 +++ ...PhabricatorAjaxRequestExceptionHandler.php | 38 ++++ ...bricatorConduitRequestExceptionHandler.php | 33 +++ ...bricatorDefaultRequestExceptionHandler.php | 76 +++++++ ...torHighSecurityRequestExceptionHandler.php | 76 +++++++ ...abricatorPolicyRequestExceptionHandler.php | 93 ++++++++ ...icatorRateLimitRequestExceptionHandler.php | 42 ++++ .../PhabricatorRequestExceptionHandler.php | 26 +++ .../module/PhabricatorConfigEdgeModule.php | 2 +- .../module/PhabricatorConfigPHIDModule.php | 2 +- ...torConfigRequestExceptionHandlerModule.php | 47 ++++ .../module/PhabricatorConfigSiteModule.php | 2 +- 15 files changed, 550 insertions(+), 218 deletions(-) create mode 100644 src/aphront/handler/AphrontRequestExceptionHandler.php create mode 100644 src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php create mode 100644 src/aphront/handler/PhabricatorConduitRequestExceptionHandler.php create mode 100644 src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php create mode 100644 src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php create mode 100644 src/aphront/handler/PhabricatorPolicyRequestExceptionHandler.php create mode 100644 src/aphront/handler/PhabricatorRateLimitRequestExceptionHandler.php create mode 100644 src/aphront/handler/PhabricatorRequestExceptionHandler.php create mode 100644 src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9bde309819..262d144ecd 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -157,6 +157,7 @@ phutil_register_library_map(array( 'AphrontRedirectResponseTestCase' => 'aphront/response/__tests__/AphrontRedirectResponseTestCase.php', 'AphrontReloadResponse' => 'aphront/response/AphrontReloadResponse.php', 'AphrontRequest' => 'aphront/AphrontRequest.php', + 'AphrontRequestExceptionHandler' => 'aphront/handler/AphrontRequestExceptionHandler.php', 'AphrontRequestTestCase' => 'aphront/__tests__/AphrontRequestTestCase.php', 'AphrontResponse' => 'aphront/response/AphrontResponse.php', 'AphrontResponseProducerInterface' => 'aphront/interface/AphrontResponseProducerInterface.php', @@ -1484,6 +1485,7 @@ phutil_register_library_map(array( 'PhabricatorActionView' => 'view/layout/PhabricatorActionView.php', 'PhabricatorActivitySettingsPanel' => 'applications/settings/panel/PhabricatorActivitySettingsPanel.php', 'PhabricatorAdministratorsPolicyRule' => 'applications/policy/rule/PhabricatorAdministratorsPolicyRule.php', + 'PhabricatorAjaxRequestExceptionHandler' => 'aphront/handler/PhabricatorAjaxRequestExceptionHandler.php', 'PhabricatorAlmanacApplication' => 'applications/almanac/application/PhabricatorAlmanacApplication.php', 'PhabricatorAmazonAuthProvider' => 'applications/auth/provider/PhabricatorAmazonAuthProvider.php', 'PhabricatorAnchorView' => 'view/layout/PhabricatorAnchorView.php', @@ -1786,6 +1788,7 @@ phutil_register_library_map(array( 'PhabricatorConduitLogQuery' => 'applications/conduit/query/PhabricatorConduitLogQuery.php', 'PhabricatorConduitMethodCallLog' => 'applications/conduit/storage/PhabricatorConduitMethodCallLog.php', 'PhabricatorConduitMethodQuery' => 'applications/conduit/query/PhabricatorConduitMethodQuery.php', + 'PhabricatorConduitRequestExceptionHandler' => 'aphront/handler/PhabricatorConduitRequestExceptionHandler.php', 'PhabricatorConduitSearchEngine' => 'applications/conduit/query/PhabricatorConduitSearchEngine.php', 'PhabricatorConduitTestCase' => '__tests__/PhabricatorConduitTestCase.php', 'PhabricatorConduitToken' => 'applications/conduit/storage/PhabricatorConduitToken.php', @@ -1838,6 +1841,7 @@ phutil_register_library_map(array( 'PhabricatorConfigOptionType' => 'applications/config/custom/PhabricatorConfigOptionType.php', 'PhabricatorConfigPHIDModule' => 'applications/config/module/PhabricatorConfigPHIDModule.php', 'PhabricatorConfigProxySource' => 'infrastructure/env/PhabricatorConfigProxySource.php', + 'PhabricatorConfigRequestExceptionHandlerModule' => 'applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php', 'PhabricatorConfigResponse' => 'applications/config/response/PhabricatorConfigResponse.php', 'PhabricatorConfigSchemaQuery' => 'applications/config/schema/PhabricatorConfigSchemaQuery.php', 'PhabricatorConfigSchemaSpec' => 'applications/config/schema/PhabricatorConfigSchemaSpec.php', @@ -1993,6 +1997,7 @@ phutil_register_library_map(array( 'PhabricatorDatabaseSetupCheck' => 'applications/config/check/PhabricatorDatabaseSetupCheck.php', 'PhabricatorDateTimeSettingsPanel' => 'applications/settings/panel/PhabricatorDateTimeSettingsPanel.php', 'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php', + 'PhabricatorDefaultRequestExceptionHandler' => 'aphront/handler/PhabricatorDefaultRequestExceptionHandler.php', 'PhabricatorDesktopNotificationsSettingsPanel' => 'applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php', 'PhabricatorDestructibleInterface' => 'applications/system/interface/PhabricatorDestructibleInterface.php', 'PhabricatorDestructionEngine' => 'applications/system/engine/PhabricatorDestructionEngine.php', @@ -2183,6 +2188,7 @@ phutil_register_library_map(array( 'PhabricatorHelpEditorProtocolController' => 'applications/help/controller/PhabricatorHelpEditorProtocolController.php', 'PhabricatorHelpKeyboardShortcutController' => 'applications/help/controller/PhabricatorHelpKeyboardShortcutController.php', 'PhabricatorHeraldApplication' => 'applications/herald/application/PhabricatorHeraldApplication.php', + 'PhabricatorHighSecurityRequestExceptionHandler' => 'aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php', 'PhabricatorHomeApplication' => 'applications/home/application/PhabricatorHomeApplication.php', 'PhabricatorHomeController' => 'applications/home/controller/PhabricatorHomeController.php', 'PhabricatorHomeMainController' => 'applications/home/controller/PhabricatorHomeMainController.php', @@ -2580,6 +2586,7 @@ phutil_register_library_map(array( 'PhabricatorPolicyManagementWorkflow' => 'applications/policy/management/PhabricatorPolicyManagementWorkflow.php', 'PhabricatorPolicyPHIDTypePolicy' => 'applications/policy/phid/PhabricatorPolicyPHIDTypePolicy.php', 'PhabricatorPolicyQuery' => 'applications/policy/query/PhabricatorPolicyQuery.php', + 'PhabricatorPolicyRequestExceptionHandler' => 'aphront/handler/PhabricatorPolicyRequestExceptionHandler.php', 'PhabricatorPolicyRule' => 'applications/policy/rule/PhabricatorPolicyRule.php', 'PhabricatorPolicyTestCase' => 'applications/policy/__tests__/PhabricatorPolicyTestCase.php', 'PhabricatorPolicyTestObject' => 'applications/policy/__tests__/PhabricatorPolicyTestObject.php', @@ -2667,6 +2674,7 @@ phutil_register_library_map(array( 'PhabricatorQueryOrderItem' => 'infrastructure/query/order/PhabricatorQueryOrderItem.php', 'PhabricatorQueryOrderTestCase' => 'infrastructure/query/order/__tests__/PhabricatorQueryOrderTestCase.php', 'PhabricatorQueryOrderVector' => 'infrastructure/query/order/PhabricatorQueryOrderVector.php', + 'PhabricatorRateLimitRequestExceptionHandler' => 'aphront/handler/PhabricatorRateLimitRequestExceptionHandler.php', 'PhabricatorRecaptchaConfigOptions' => 'applications/config/option/PhabricatorRecaptchaConfigOptions.php', 'PhabricatorRecipientHasBadgeEdgeType' => 'applications/badges/edge/PhabricatorRecipientHasBadgeEdgeType.php', 'PhabricatorRedirectController' => 'applications/base/controller/PhabricatorRedirectController.php', @@ -2758,6 +2766,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryURITestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryURITestCase.php', 'PhabricatorRepositoryVCSPassword' => 'applications/repository/storage/PhabricatorRepositoryVCSPassword.php', 'PhabricatorRepositoryVersion' => 'applications/repository/constants/PhabricatorRepositoryVersion.php', + 'PhabricatorRequestExceptionHandler' => 'aphront/handler/PhabricatorRequestExceptionHandler.php', 'PhabricatorResourceSite' => 'aphront/site/PhabricatorResourceSite.php', 'PhabricatorRobotsController' => 'applications/system/controller/PhabricatorRobotsController.php', 'PhabricatorS3FileStorageEngine' => 'applications/files/engine/PhabricatorS3FileStorageEngine.php', @@ -3788,6 +3797,7 @@ phutil_register_library_map(array( 'AphrontRedirectResponseTestCase' => 'PhabricatorTestCase', 'AphrontReloadResponse' => 'AphrontRedirectResponse', 'AphrontRequest' => 'Phobject', + 'AphrontRequestExceptionHandler' => 'Phobject', 'AphrontRequestTestCase' => 'PhabricatorTestCase', 'AphrontResponse' => 'Phobject', 'AphrontRoutingMap' => 'Phobject', @@ -5303,6 +5313,7 @@ phutil_register_library_map(array( 'PhabricatorActionView' => 'AphrontView', 'PhabricatorActivitySettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorAdministratorsPolicyRule' => 'PhabricatorPolicyRule', + 'PhabricatorAjaxRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorAlmanacApplication' => 'PhabricatorApplication', 'PhabricatorAmazonAuthProvider' => 'PhabricatorOAuth2AuthProvider', 'PhabricatorAnchorView' => 'AphrontView', @@ -5667,6 +5678,7 @@ phutil_register_library_map(array( 'PhabricatorPolicyInterface', ), 'PhabricatorConduitMethodQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorConduitRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorConduitSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorConduitTestCase' => 'PhabricatorTestCase', 'PhabricatorConduitToken' => array( @@ -5729,6 +5741,7 @@ phutil_register_library_map(array( 'PhabricatorConfigOptionType' => 'Phobject', 'PhabricatorConfigPHIDModule' => 'PhabricatorConfigModule', 'PhabricatorConfigProxySource' => 'PhabricatorConfigSource', + 'PhabricatorConfigRequestExceptionHandlerModule' => 'PhabricatorConfigModule', 'PhabricatorConfigResponse' => 'AphrontStandaloneHTMLResponse', 'PhabricatorConfigSchemaQuery' => 'Phobject', 'PhabricatorConfigSchemaSpec' => 'Phobject', @@ -5913,6 +5926,7 @@ phutil_register_library_map(array( 'PhabricatorDatabaseSetupCheck' => 'PhabricatorSetupCheck', 'PhabricatorDateTimeSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorDebugController' => 'PhabricatorController', + 'PhabricatorDefaultRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorDesktopNotificationsSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorDestructionEngine' => 'Phobject', 'PhabricatorDeveloperConfigOptions' => 'PhabricatorApplicationConfigOptions', @@ -6138,6 +6152,7 @@ phutil_register_library_map(array( 'PhabricatorHelpEditorProtocolController' => 'PhabricatorHelpController', 'PhabricatorHelpKeyboardShortcutController' => 'PhabricatorHelpController', 'PhabricatorHeraldApplication' => 'PhabricatorApplication', + 'PhabricatorHighSecurityRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorHomeApplication' => 'PhabricatorApplication', 'PhabricatorHomeController' => 'PhabricatorController', 'PhabricatorHomeMainController' => 'PhabricatorHomeController', @@ -6587,6 +6602,7 @@ phutil_register_library_map(array( 'PhabricatorPolicyManagementWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorPolicyPHIDTypePolicy' => 'PhabricatorPHIDType', 'PhabricatorPolicyQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorPolicyRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorPolicyRule' => 'Phobject', 'PhabricatorPolicyTestCase' => 'PhabricatorTestCase', 'PhabricatorPolicyTestObject' => array( @@ -6702,6 +6718,7 @@ phutil_register_library_map(array( 'Phobject', 'Iterator', ), + 'PhabricatorRateLimitRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler', 'PhabricatorRecaptchaConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorRecipientHasBadgeEdgeType' => 'PhabricatorEdgeType', 'PhabricatorRedirectController' => 'PhabricatorController', @@ -6828,6 +6845,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryURITestCase' => 'PhabricatorTestCase', 'PhabricatorRepositoryVCSPassword' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryVersion' => 'Phobject', + 'PhabricatorRequestExceptionHandler' => 'AphrontRequestExceptionHandler', 'PhabricatorResourceSite' => 'PhabricatorSite', 'PhabricatorRobotsController' => 'PhabricatorController', 'PhabricatorS3FileStorageEngine' => 'PhabricatorFileStorageEngine', diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index dad223a63f..50b8c123fd 100644 --- a/src/aphront/configuration/AphrontApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontApplicationConfiguration.php @@ -3,6 +3,7 @@ /** * @task routing URI Routing * @task response Response Handling + * @task exception Exception Handling */ abstract class AphrontApplicationConfiguration extends Phobject { @@ -11,7 +12,6 @@ abstract class AphrontApplicationConfiguration extends Phobject { private $path; private $console; - abstract public function getApplicationName(); abstract public function buildRequest(); abstract public function build404Controller(); abstract public function buildRedirectController($uri, $external); @@ -482,7 +482,7 @@ abstract class AphrontApplicationConfiguration extends Phobject { /** - * Verifies that the erturn value from an + * Verifies that the return value from an * @{class:AphrontResponseProducerInterface} is of an allowed type. * * @param AphrontResponseProducerInterface Object which produced @@ -511,6 +511,36 @@ abstract class AphrontApplicationConfiguration extends Phobject { } + /** + * Verifies that the return value from an + * @{class:AphrontRequestExceptionHandler} is of an allowed type. + * + * @param AphrontRequestExceptionHandler Object which produced this + * response. + * @param wild Supposedly valid response. + * @return void + * @task response + */ + private function validateErrorHandlerResponse( + AphrontRequestExceptionHandler $handler, + $response) { + + if ($this->isValidResponseObject($response)) { + return; + } + + throw new Exception( + pht( + 'Exception handler "%s" returned an invalid response from call to '. + '"%s". This method must return an object of class "%s", or an object '. + 'which implements the "%s" interface.', + get_class($handler), + 'handleRequestException()', + 'AphrontResponse', + 'AphrontResponseProducerInterface')); + } + + /** * Resolves a response object into an @{class:AphrontResponse}. * @@ -572,4 +602,34 @@ abstract class AphrontApplicationConfiguration extends Phobject { } +/* -( Error Handling )----------------------------------------------------- */ + + + /** + * Convert an exception which has escaped the controller into a response. + * + * This method delegates exception handling to available subclasses of + * @{class:AphrontRequestExceptionHandler}. + * + * @param Exception Exception which needs to be handled. + * @return wild Response or response producer, or null if no available + * handler can produce a response. + * @task exception + */ + private function handleException(Exception $ex) { + $handlers = AphrontRequestExceptionHandler::getAllHandlers(); + + $request = $this->getRequest(); + foreach ($handlers as $handler) { + if ($handler->canHandleRequestException($request, $ex)) { + $response = $handler->handleRequestException($request, $ex); + $this->validateErrorHandlerResponse($handler, $response); + return $response; + } + } + + throw $ex; + } + + } diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php index c51aac3749..cd1347411a 100644 --- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php @@ -8,12 +8,6 @@ class AphrontDefaultApplicationConfiguration extends AphrontApplicationConfiguration { - public function __construct() {} - - public function getApplicationName() { - return 'aphront-default'; - } - /** * @phutil-external-symbol class PhabricatorStartup */ @@ -50,213 +44,6 @@ class AphrontDefaultApplicationConfiguration return $request; } - public function handleException(Exception $ex) { - $request = $this->getRequest(); - - // For Conduit requests, return a Conduit response. - if ($request->isConduit()) { - $response = new ConduitAPIResponse(); - $response->setErrorCode(get_class($ex)); - $response->setErrorInfo($ex->getMessage()); - - return id(new AphrontJSONResponse()) - ->setAddJSONShield(false) - ->setContent($response->toDictionary()); - } - - // For non-workflow requests, return a Ajax response. - if ($request->isAjax() && !$request->isWorkflow()) { - // Log these; they don't get shown on the client and can be difficult - // to debug. - phlog($ex); - - $response = new AphrontAjaxResponse(); - $response->setError( - array( - 'code' => get_class($ex), - 'info' => $ex->getMessage(), - )); - return $response; - } - - $user = $request->getUser(); - if (!$user) { - // If we hit an exception very early, we won't have a user. - $user = new PhabricatorUser(); - } - - if ($ex instanceof PhabricatorSystemActionRateLimitException) { - $dialog = id(new AphrontDialogView()) - ->setTitle(pht('Slow Down!')) - ->setUser($user) - ->setErrors(array(pht('You are being rate limited.'))) - ->appendParagraph($ex->getMessage()) - ->appendParagraph($ex->getRateExplanation()) - ->addCancelButton('/', pht('Okaaaaaaaaaaaaaay...')); - - $response = new AphrontDialogResponse(); - $response->setDialog($dialog); - return $response; - } - - if ($ex instanceof PhabricatorAuthHighSecurityRequiredException) { - - $form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm( - $ex->getFactors(), - $ex->getFactorValidationResults(), - $user, - $request); - - $dialog = id(new AphrontDialogView()) - ->setUser($user) - ->setTitle(pht('Entering High Security')) - ->setShortTitle(pht('Security Checkpoint')) - ->setWidth(AphrontDialogView::WIDTH_FORM) - ->addHiddenInput(AphrontRequest::TYPE_HISEC, true) - ->setErrors( - array( - pht( - 'You are taking an action which requires you to enter '. - 'high security.'), - )) - ->appendParagraph( - pht( - 'High security mode helps protect your account from security '. - 'threats, like session theft or someone messing with your stuff '. - 'while you\'re grabbing a coffee. To enter high security mode, '. - 'confirm your credentials.')) - ->appendChild($form->buildLayoutView()) - ->appendParagraph( - pht( - 'Your account will remain in high security mode for a short '. - 'period of time. When you are finished taking sensitive '. - 'actions, you should leave high security.')) - ->setSubmitURI($request->getPath()) - ->addCancelButton($ex->getCancelURI()) - ->addSubmitButton(pht('Enter High Security')); - - $request_parameters = $request->getPassthroughRequestParameters( - $respect_quicksand = true); - foreach ($request_parameters as $key => $value) { - $dialog->addHiddenInput($key, $value); - } - - $response = new AphrontDialogResponse(); - $response->setDialog($dialog); - return $response; - } - - if ($ex instanceof PhabricatorPolicyException) { - if (!$user->isLoggedIn()) { - // If the user isn't logged in, just give them a login form. This is - // probably a generally more useful response than a policy dialog that - // they have to click through to get a login form. - // - // Possibly we should add a header here like "you need to login to see - // the thing you are trying to look at". - $login_controller = new PhabricatorAuthStartController(); - $login_controller->setRequest($request); - - $auth_app_class = 'PhabricatorAuthApplication'; - $auth_app = PhabricatorApplication::getByClass($auth_app_class); - $login_controller->setCurrentApplication($auth_app); - - return $login_controller->handleRequest($request); - } - - $content = array( - phutil_tag( - 'div', - array( - 'class' => 'aphront-policy-rejection', - ), - $ex->getRejection()), - ); - - $list = null; - if ($ex->getCapabilityName()) { - $list = $ex->getMoreInfo(); - foreach ($list as $key => $item) { - $list[$key] = $item; - } - - $content[] = phutil_tag( - 'div', - array( - 'class' => 'aphront-capability-details', - ), - pht('Users with the "%s" capability:', $ex->getCapabilityName())); - - } - - $dialog = id(new AphrontDialogView()) - ->setTitle($ex->getTitle()) - ->setClass('aphront-access-dialog') - ->setUser($user) - ->appendChild($content); - - if ($list) { - $dialog->appendList($list); - } - - if ($this->getRequest()->isAjax()) { - $dialog->addCancelButton('/', pht('Close')); - } else { - $dialog->addCancelButton('/', pht('OK')); - } - - $response = new AphrontDialogResponse(); - $response->setDialog($dialog); - return $response; - } - - // Always log the unhandled exception. - phlog($ex); - - $class = get_class($ex); - $message = $ex->getMessage(); - - if ($ex instanceof AphrontSchemaQueryException) { - $message .= "\n\n".pht( - "NOTE: This usually indicates that the MySQL schema has not been ". - "properly upgraded. Run '%s' to ensure your schema is up to date.", - 'bin/storage upgrade'); - } - - if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { - $trace = id(new AphrontStackTraceView()) - ->setUser($user) - ->setTrace($ex->getTrace()); - } else { - $trace = null; - } - - $content = phutil_tag( - 'div', - array('class' => 'aphront-unhandled-exception'), - array( - phutil_tag('div', array('class' => 'exception-message'), $message), - $trace, - )); - - $dialog = new AphrontDialogView(); - $dialog - ->setTitle(pht('Unhandled Exception ("%s")', $class)) - ->setClass('aphront-exception-dialog') - ->setUser($user) - ->appendChild($content); - - if ($this->getRequest()->isAjax()) { - $dialog->addCancelButton('/', pht('Close')); - } - - $response = new AphrontDialogResponse(); - $response->setDialog($dialog); - $response->setHTTPResponseCode(500); - - return $response; - } - public function build404Controller() { return array(new Phabricator404Controller(), array()); } diff --git a/src/aphront/handler/AphrontRequestExceptionHandler.php b/src/aphront/handler/AphrontRequestExceptionHandler.php new file mode 100644 index 0000000000..694071e1ba --- /dev/null +++ b/src/aphront/handler/AphrontRequestExceptionHandler.php @@ -0,0 +1,36 @@ +setAncestorClass(__CLASS__) + ->setSortMethod('getRequestExceptionHandlerPriority') + ->execute(); + } + +} diff --git a/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php b/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php new file mode 100644 index 0000000000..06023e02de --- /dev/null +++ b/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php @@ -0,0 +1,38 @@ +isAjax() && !$request->isWorkflow()); + } + + public function handleRequestException( + AphrontRequest $request, + Exception $ex) { + + // Log these; they don't get shown on the client and can be difficult + // to debug. + phlog($ex); + + $response = new AphrontAjaxResponse(); + $response->setError( + array( + 'code' => get_class($ex), + 'info' => $ex->getMessage(), + )); + return $response; + } + +} diff --git a/src/aphront/handler/PhabricatorConduitRequestExceptionHandler.php b/src/aphront/handler/PhabricatorConduitRequestExceptionHandler.php new file mode 100644 index 0000000000..367f607f83 --- /dev/null +++ b/src/aphront/handler/PhabricatorConduitRequestExceptionHandler.php @@ -0,0 +1,33 @@ +isConduit(); + } + + public function handleRequestException( + AphrontRequest $request, + Exception $ex) { + + $response = id(new ConduitAPIResponse()) + ->setErrorCode(get_class($ex)) + ->setErrorInfo($ex->getMessage()); + + return id(new AphrontJSONResponse()) + ->setAddJSONShield(false) + ->setContent($response->toDictionary()); + } + +} diff --git a/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php b/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php new file mode 100644 index 0000000000..321dd6d69b --- /dev/null +++ b/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php @@ -0,0 +1,76 @@ +isPhabricatorSite($request)) { + return false; + } + + return true; + } + + public function handleRequestException( + AphrontRequest $request, + Exception $ex) { + + $viewer = $this->getViewer($request); + + // Always log the unhandled exception. + phlog($ex); + + $class = get_class($ex); + $message = $ex->getMessage(); + + if ($ex instanceof AphrontSchemaQueryException) { + $message .= "\n\n".pht( + "NOTE: This usually indicates that the MySQL schema has not been ". + "properly upgraded. Run '%s' to ensure your schema is up to date.", + 'bin/storage upgrade'); + } + + if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) { + $trace = id(new AphrontStackTraceView()) + ->setUser($viewer) + ->setTrace($ex->getTrace()); + } else { + $trace = null; + } + + $content = phutil_tag( + 'div', + array('class' => 'aphront-unhandled-exception'), + array( + phutil_tag('div', array('class' => 'exception-message'), $message), + $trace, + )); + + $dialog = new AphrontDialogView(); + $dialog + ->setTitle(pht('Unhandled Exception ("%s")', $class)) + ->setClass('aphront-exception-dialog') + ->setUser($viewer) + ->appendChild($content); + + if ($request->isAjax()) { + $dialog->addCancelButton('/', pht('Close')); + } + + return id(new AphrontDialogResponse()) + ->setDialog($dialog) + ->setHTTPResponseCode(500); + } + +} diff --git a/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php b/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php new file mode 100644 index 0000000000..ff4ace2e4b --- /dev/null +++ b/src/aphront/handler/PhabricatorHighSecurityRequestExceptionHandler.php @@ -0,0 +1,76 @@ +isPhabricatorSite($request)) { + return false; + } + + return ($ex instanceof PhabricatorAuthHighSecurityRequiredException); + } + + public function handleRequestException( + AphrontRequest $request, + Exception $ex) { + + $viewer = $this->getViewer($request); + + $form = id(new PhabricatorAuthSessionEngine())->renderHighSecurityForm( + $ex->getFactors(), + $ex->getFactorValidationResults(), + $viewer, + $request); + + $dialog = id(new AphrontDialogView()) + ->setUser($viewer) + ->setTitle(pht('Entering High Security')) + ->setShortTitle(pht('Security Checkpoint')) + ->setWidth(AphrontDialogView::WIDTH_FORM) + ->addHiddenInput(AphrontRequest::TYPE_HISEC, true) + ->setErrors( + array( + pht( + 'You are taking an action which requires you to enter '. + 'high security.'), + )) + ->appendParagraph( + pht( + 'High security mode helps protect your account from security '. + 'threats, like session theft or someone messing with your stuff '. + 'while you\'re grabbing a coffee. To enter high security mode, '. + 'confirm your credentials.')) + ->appendChild($form->buildLayoutView()) + ->appendParagraph( + pht( + 'Your account will remain in high security mode for a short '. + 'period of time. When you are finished taking sensitive '. + 'actions, you should leave high security.')) + ->setSubmitURI($request->getPath()) + ->addCancelButton($ex->getCancelURI()) + ->addSubmitButton(pht('Enter High Security')); + + $request_parameters = $request->getPassthroughRequestParameters( + $respect_quicksand = true); + foreach ($request_parameters as $key => $value) { + $dialog->addHiddenInput($key, $value); + } + + return $dialog; + } + +} diff --git a/src/aphront/handler/PhabricatorPolicyRequestExceptionHandler.php b/src/aphront/handler/PhabricatorPolicyRequestExceptionHandler.php new file mode 100644 index 0000000000..1c84e90fd1 --- /dev/null +++ b/src/aphront/handler/PhabricatorPolicyRequestExceptionHandler.php @@ -0,0 +1,93 @@ +isPhabricatorSite($request)) { + return false; + } + + return ($ex instanceof PhabricatorPolicyException); + } + + public function handleRequestException( + AphrontRequest $request, + Exception $ex) { + + $viewer = $this->getViewer($request); + + if (!$viewer->isLoggedIn()) { + // If the user isn't logged in, just give them a login form. This is + // probably a generally more useful response than a policy dialog that + // they have to click through to get a login form. + // + // Possibly we should add a header here like "you need to login to see + // the thing you are trying to look at". + $auth_app_class = 'PhabricatorAuthApplication'; + $auth_app = PhabricatorApplication::getByClass($auth_app_class); + + return id(new PhabricatorAuthStartController()) + ->setRequest($request) + ->setCurrentApplication($auth_app) + ->handleRequest($request); + } + + $content = array( + phutil_tag( + 'div', + array( + 'class' => 'aphront-policy-rejection', + ), + $ex->getRejection()), + ); + + $list = null; + if ($ex->getCapabilityName()) { + $list = $ex->getMoreInfo(); + foreach ($list as $key => $item) { + $list[$key] = $item; + } + + $content[] = phutil_tag( + 'div', + array( + 'class' => 'aphront-capability-details', + ), + pht('Users with the "%s" capability:', $ex->getCapabilityName())); + + } + + $dialog = id(new AphrontDialogView()) + ->setTitle($ex->getTitle()) + ->setClass('aphront-access-dialog') + ->setUser($viewer) + ->appendChild($content); + + if ($list) { + $dialog->appendList($list); + } + + if ($request->isAjax()) { + $dialog->addCancelButton('/', pht('Close')); + } else { + $dialog->addCancelButton('/', pht('OK')); + } + + return $dialog; + } + +} diff --git a/src/aphront/handler/PhabricatorRateLimitRequestExceptionHandler.php b/src/aphront/handler/PhabricatorRateLimitRequestExceptionHandler.php new file mode 100644 index 0000000000..3fb549c29f --- /dev/null +++ b/src/aphront/handler/PhabricatorRateLimitRequestExceptionHandler.php @@ -0,0 +1,42 @@ +isPhabricatorSite($request)) { + return false; + } + + return ($ex instanceof PhabricatorSystemActionRateLimitException); + } + + public function handleRequestException( + AphrontRequest $request, + Exception $ex) { + + $viewer = $this->getViewer($request); + + return id(new AphrontDialogView()) + ->setTitle(pht('Slow Down!')) + ->setUser($viewer) + ->setErrors(array(pht('You are being rate limited.'))) + ->appendParagraph($ex->getMessage()) + ->appendParagraph($ex->getRateExplanation()) + ->addCancelButton('/', pht('Okaaaaaaaaaaaaaay...')); + } + +} diff --git a/src/aphront/handler/PhabricatorRequestExceptionHandler.php b/src/aphront/handler/PhabricatorRequestExceptionHandler.php new file mode 100644 index 0000000000..81e21b9107 --- /dev/null +++ b/src/aphront/handler/PhabricatorRequestExceptionHandler.php @@ -0,0 +1,26 @@ +getSite(); + if (!$site) { + return false; + } + + return ($site instanceof PhabricatorSite); + } + + protected function getViewer(AphrontRequest $request) { + $viewer = $request->getUser(); + + if ($viewer) { + return $viewer; + } + + // If we hit an exception very early, we won't have a user yet. + return new PhabricatorUser(); + } + +} diff --git a/src/applications/config/module/PhabricatorConfigEdgeModule.php b/src/applications/config/module/PhabricatorConfigEdgeModule.php index f8a348caf3..8aa5d74664 100644 --- a/src/applications/config/module/PhabricatorConfigEdgeModule.php +++ b/src/applications/config/module/PhabricatorConfigEdgeModule.php @@ -41,7 +41,7 @@ final class PhabricatorConfigEdgeModule extends PhabricatorConfigModule { return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Edge Types')) - ->appendChild($table); + ->setTable($table); } } diff --git a/src/applications/config/module/PhabricatorConfigPHIDModule.php b/src/applications/config/module/PhabricatorConfigPHIDModule.php index 300740ea77..6c363e3bee 100644 --- a/src/applications/config/module/PhabricatorConfigPHIDModule.php +++ b/src/applications/config/module/PhabricatorConfigPHIDModule.php @@ -41,7 +41,7 @@ final class PhabricatorConfigPHIDModule extends PhabricatorConfigModule { return id(new PHUIObjectBoxView()) ->setHeaderText(pht('PHID Types')) - ->appendChild($table); + ->setTable($table); } } diff --git a/src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php b/src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php new file mode 100644 index 0000000000..22539a1bd3 --- /dev/null +++ b/src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php @@ -0,0 +1,47 @@ +getViewer(); + + $handlers = AphrontRequestExceptionHandler::getAllHandlers(); + + $rows = array(); + foreach ($handlers as $key => $handler) { + $rows[] = array( + $handler->getRequestExceptionHandlerPriority(), + $key, + $handler->getRequestExceptionHandlerDescription(), + ); + } + + $table = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Priority'), + pht('Class'), + pht('Description'), + )) + ->setColumnClasses( + array( + null, + 'pri', + 'wide', + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Exception Handlers')) + ->setTable($table); + } + +} diff --git a/src/applications/config/module/PhabricatorConfigSiteModule.php b/src/applications/config/module/PhabricatorConfigSiteModule.php index 0b85db2a10..3f63ae2da4 100644 --- a/src/applications/config/module/PhabricatorConfigSiteModule.php +++ b/src/applications/config/module/PhabricatorConfigSiteModule.php @@ -40,7 +40,7 @@ final class PhabricatorConfigSiteModule extends PhabricatorConfigModule { return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Sites')) - ->appendChild($table); + ->setTable($table); } }