Page MenuHomePhabricator
Files F14006301

D14047.diff
No OneTemporary
Actions

D14047.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -143,6 +143,7 @@
'AphrontJavelinView' => 'view/AphrontJavelinView.php',
'AphrontKeyboardShortcutsAvailableView' => 'view/widget/AphrontKeyboardShortcutsAvailableView.php',
'AphrontListFilterView' => 'view/layout/AphrontListFilterView.php',
+ 'AphrontMalformedRequestException' => 'aphront/exception/AphrontMalformedRequestException.php',
'AphrontMoreView' => 'view/layout/AphrontMoreView.php',
'AphrontMultiColumnView' => 'view/layout/AphrontMultiColumnView.php',
'AphrontMySQLDatabaseConnectionTestCase' => 'infrastructure/storage/__tests__/AphrontMySQLDatabaseConnectionTestCase.php',
@@ -170,7 +171,6 @@
'AphrontTokenizerTemplateView' => 'view/control/AphrontTokenizerTemplateView.php',
'AphrontTypeaheadTemplateView' => 'view/control/AphrontTypeaheadTemplateView.php',
'AphrontUnhandledExceptionResponse' => 'aphront/response/AphrontUnhandledExceptionResponse.php',
- 'AphrontUsageException' => 'aphront/exception/AphrontUsageException.php',
'AphrontView' => 'view/AphrontView.php',
'AphrontWebpageResponse' => 'aphront/response/AphrontWebpageResponse.php',
'ArcanistConduitAPIMethod' => 'applications/arcanist/conduit/ArcanistConduitAPIMethod.php',
@@ -3771,6 +3771,7 @@
'AphrontJavelinView' => 'AphrontView',
'AphrontKeyboardShortcutsAvailableView' => 'AphrontView',
'AphrontListFilterView' => 'AphrontView',
+ 'AphrontMalformedRequestException' => 'AphrontException',
'AphrontMoreView' => 'AphrontView',
'AphrontMultiColumnView' => 'AphrontView',
'AphrontMySQLDatabaseConnectionTestCase' => 'PhabricatorTestCase',
@@ -3800,7 +3801,6 @@
'AphrontTokenizerTemplateView' => 'AphrontView',
'AphrontTypeaheadTemplateView' => 'AphrontView',
'AphrontUnhandledExceptionResponse' => 'AphrontStandaloneHTMLResponse',
- 'AphrontUsageException' => 'AphrontException',
'AphrontView' => array(
'Phobject',
'PhutilSafeHTMLProducerInterface',
@@ -4383,7 +4383,7 @@
'DiffusionSearchQueryConduitAPIMethod' => 'DiffusionQueryConduitAPIMethod',
'DiffusionServeController' => 'DiffusionController',
'DiffusionSetPasswordSettingsPanel' => 'PhabricatorSettingsPanel',
- 'DiffusionSetupException' => 'AphrontUsageException',
+ 'DiffusionSetupException' => 'Exception',
'DiffusionSubversionSSHWorkflow' => 'DiffusionSSHWorkflow',
'DiffusionSubversionServeSSHWorkflow' => 'DiffusionSubversionSSHWorkflow',
'DiffusionSubversionWireProtocol' => 'Phobject',
diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@@ -318,7 +318,7 @@
// This is a command line script (probably something like a unit
// test) so it's fine that we don't have SERVER_ADDR defined.
} else {
- throw new AphrontUsageException(
+ throw new AphrontMalformedRequestException(
pht('No %s', 'SERVER_ADDR'),
pht(
'Phabricator is configured to operate in cluster mode, but '.
@@ -330,7 +330,7 @@
}
} else {
if (!PhabricatorEnv::isClusterAddress($server_addr)) {
- throw new AphrontUsageException(
+ throw new AphrontMalformedRequestException(
pht('External Interface'),
pht(
'Phabricator is configured in cluster mode and the address '.
@@ -413,12 +413,14 @@
if (!$site) {
$path = $request->getPath();
$host = $request->getHost();
- throw new Exception(
+ throw new AphrontMalformedRequestException(
+ pht('Site Not Found'),
pht(
'This request asked for "%s" on host "%s", but no site is '.
'configured which can serve this request.',
$path,
- $host));
+ $host),
+ true);
}
$request->setSite($site);
diff --git a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
--- a/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontDefaultApplicationConfiguration.php
@@ -210,22 +210,6 @@
return $response;
}
- if ($ex instanceof AphrontUsageException) {
- $error = new PHUIInfoView();
- $error->setTitle($ex->getTitle());
- $error->appendChild($ex->getMessage());
-
- $view = new PhabricatorStandardPageView();
- $view->setRequest($this->getRequest());
- $view->appendChild($error);
-
- $response = new AphrontWebpageResponse();
- $response->setContent($view->render());
- $response->setHTTPResponseCode(500);
-
- return $response;
- }
-
// Always log the unhandled exception.
phlog($ex);
diff --git a/src/aphront/exception/AphrontMalformedRequestException.php b/src/aphront/exception/AphrontMalformedRequestException.php
new file mode 100644
--- /dev/null
+++ b/src/aphront/exception/AphrontMalformedRequestException.php
@@ -0,0 +1,34 @@
+<?php
+
+/**
+ * These exceptions are raised when a client submits a malformed request.
+ *
+ * These errors are caught by Aphront itself and occur too early or too
+ * fundamentally in request handling to allow the request to route to a
+ * controller or survive to normal processing.
+ *
+ * These exceptions can be made "unlogged", which will prevent them from being
+ * logged. The intent is that errors which are purely the result of client
+ * failure and of no interest to the server can be raised silently to avoid
+ * cluttering the logs with client errors that are not actionable.
+ */
+final class AphrontMalformedRequestException extends AphrontException {
+
+ private $title;
+ private $isUnlogged;
+
+ public function __construct($title, $message, $unlogged = false) {
+ $this->title = $title;
+ $this->isUnlogged = $unlogged;
+ parent::__construct($message);
+ }
+
+ public function getTitle() {
+ return $this->title;
+ }
+
+ public function getIsUnlogged() {
+ return $this->isUnlogged;
+ }
+
+}
diff --git a/src/aphront/exception/AphrontUsageException.php b/src/aphront/exception/AphrontUsageException.php
deleted file mode 100644
--- a/src/aphront/exception/AphrontUsageException.php
+++ /dev/null
@@ -1,21 +0,0 @@
-<?php
-
-/**
- * These exceptions represent user error, and are not logged.
- *
- * @concrete-extensible
- */
-class AphrontUsageException extends AphrontException {
-
- private $title;
-
- public function __construct($title, $message) {
- $this->title = $title;
- parent::__construct($message);
- }
-
- public function getTitle() {
- return $this->title;
- }
-
-}
diff --git a/src/aphront/response/AphrontUnhandledExceptionResponse.php b/src/aphront/response/AphrontUnhandledExceptionResponse.php
--- a/src/aphront/response/AphrontUnhandledExceptionResponse.php
+++ b/src/aphront/response/AphrontUnhandledExceptionResponse.php
@@ -6,6 +6,20 @@
private $exception;
public function setException(Exception $exception) {
+ // Log the exception unless it's specifically a silent malformed request
+ // exception.
+
+ $should_log = true;
+ if ($exception instanceof AphrontMalformedRequestException) {
+ if ($exception->getIsUnlogged()) {
+ $should_log = false;
+ }
+ }
+
+ if ($should_log) {
+ phlog($exception);
+ }
+
$this->exception = $exception;
return $this;
}
@@ -24,7 +38,7 @@
protected function getResponseTitle() {
$ex = $this->exception;
- if ($ex instanceof AphrontUsageException) {
+ if ($ex instanceof AphrontMalformedRequestException) {
return $ex->getTitle();
} else {
return pht('Unhandled Exception');
@@ -38,7 +52,7 @@
protected function getResponseBody() {
$ex = $this->exception;
- if ($ex instanceof AphrontUsageException) {
+ if ($ex instanceof AphrontMalformedRequestException) {
$title = $ex->getTitle();
} else {
$title = get_class($ex);
diff --git a/src/applications/diffusion/exception/DiffusionSetupException.php b/src/applications/diffusion/exception/DiffusionSetupException.php
--- a/src/applications/diffusion/exception/DiffusionSetupException.php
+++ b/src/applications/diffusion/exception/DiffusionSetupException.php
@@ -1,9 +1,3 @@
<?php
-final class DiffusionSetupException extends AphrontUsageException {
-
- public function __construct($message) {
- parent::__construct(pht('Diffusion Setup Exception'), $message);
- }
-
-}
+final class DiffusionSetupException extends Exception {}

File Metadata

Mime Type
text/plain
Expires
Tue, Oct 29, 5:47 AM (3 w, 17 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6719205
Default Alt Text
D14047.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility