Page MenuHomePhabricator
Files F14037891

D16402.id39444.diff
No OneTemporary
Actions

D16402.id39444.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
@@ -132,7 +132,6 @@
'AphrontApplicationConfiguration' => 'aphront/configuration/AphrontApplicationConfiguration.php',
'AphrontBarView' => 'view/widget/bars/AphrontBarView.php',
'AphrontBoolHTTPParameterType' => 'aphront/httpparametertype/AphrontBoolHTTPParameterType.php',
- 'AphrontCSRFException' => 'aphront/exception/AphrontCSRFException.php',
'AphrontCalendarEventView' => 'applications/calendar/view/AphrontCalendarEventView.php',
'AphrontController' => 'aphront/AphrontController.php',
'AphrontCursorPagerView' => 'view/control/AphrontCursorPagerView.php',
@@ -4566,7 +4565,6 @@
'AphrontApplicationConfiguration' => 'Phobject',
'AphrontBarView' => 'AphrontView',
'AphrontBoolHTTPParameterType' => 'AphrontHTTPParameterType',
- 'AphrontCSRFException' => 'AphrontException',
'AphrontCalendarEventView' => 'AphrontView',
'AphrontController' => 'Phobject',
'AphrontCursorPagerView' => 'AphrontView',
diff --git a/src/aphront/AphrontRequest.php b/src/aphront/AphrontRequest.php
--- a/src/aphront/AphrontRequest.php
+++ b/src/aphront/AphrontRequest.php
@@ -261,25 +261,30 @@
// Add some diagnostic details so we can figure out if some CSRF issues
// are JS problems or people accessing Ajax URIs directly with their
// browsers.
- $more_info = array();
+ $info = array();
+
+ $info[] = pht(
+ 'You are trying to save some data to Phabricator, but the request '.
+ 'your browser made included an incorrect token. Reload the page '.
+ 'and try again. You may need to clear your cookies.');
if ($this->isAjax()) {
- $more_info[] = pht('This was an Ajax request.');
+ $info[] = pht('This was an Ajax request.');
} else {
- $more_info[] = pht('This was a Web request.');
+ $info[] = pht('This was a Web request.');
}
if ($token) {
- $more_info[] = pht('This request had an invalid CSRF token.');
+ $info[] = pht('This request had an invalid CSRF token.');
} else {
- $more_info[] = pht('This request had no CSRF token.');
+ $info[] = pht('This request had no CSRF token.');
}
// Give a more detailed explanation of how to avoid the exception
// in developer mode.
if (PhabricatorEnv::getEnvConfig('phabricator.developer-mode')) {
// TODO: Clean this up, see T1921.
- $more_info[] = pht(
+ $info[] = pht(
"To avoid this error, use %s to construct forms. If you are already ".
"using %s, make sure the form 'action' uses a relative URI (i.e., ".
"begins with a '%s'). Forms using absolute URIs do not include CSRF ".
@@ -299,16 +304,16 @@
'setRenderAsForm(true)');
}
+ $message = implode("\n", $info);
+
// This should only be able to happen if you load a form, pull your
// internet for 6 hours, and then reconnect and immediately submit,
// but give the user some indication of what happened since the workflow
// is incredibly confusing otherwise.
- throw new AphrontCSRFException(
- pht(
- 'You are trying to save some data to Phabricator, but the request '.
- 'your browser made included an incorrect token. Reload the page '.
- 'and try again. You may need to clear your cookies.')."\n\n".
- implode("\n", $more_info));
+ throw new AphrontMalformedRequestException(
+ pht('Invalid Request (CSRF)'),
+ $message,
+ true);
}
return true;
@@ -480,7 +485,8 @@
$configured_as = PhabricatorEnv::getEnvConfig('phabricator.base-uri');
$accessed_as = $this->getHost();
- throw new Exception(
+ throw new AphrontMalformedRequestException(
+ pht('Bad Host Header'),
pht(
'This Phabricator install is configured as "%s", but you are '.
'using the domain name "%s" to access a page which is trying to '.
@@ -488,7 +494,8 @@
'domain or a configured alternate domain. Phabricator will not '.
'set cookies on other domains for security reasons.',
$configured_as,
- $accessed_as));
+ $accessed_as),
+ true);
}
$base_domain = $base_domain_uri->getDomain();
diff --git a/src/aphront/exception/AphrontCSRFException.php b/src/aphront/exception/AphrontCSRFException.php
deleted file mode 100644
--- a/src/aphront/exception/AphrontCSRFException.php
+++ /dev/null
@@ -1,3 +0,0 @@
-<?php
-
-final class AphrontCSRFException extends AphrontException {}
diff --git a/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php b/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php
--- a/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php
+++ b/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php
@@ -28,8 +28,18 @@
$viewer = $this->getViewer($request);
- // Always log the unhandled exception.
- phlog($ex);
+ // Some types of uninteresting request exceptions don't get logged, usually
+ // because they are caused by the background radiation of bot traffic on
+ // the internet. These include requests with bad CSRF tokens and
+ // questionable "Host" headers.
+ $should_log = true;
+ if ($ex instanceof AphrontMalformedRequestException) {
+ $should_log = !$ex->getIsUnlogged();
+ }
+
+ if ($should_log) {
+ phlog($ex);
+ }
$class = get_class($ex);
$message = $ex->getMessage();
diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php
--- a/src/applications/auth/provider/PhabricatorAuthProvider.php
+++ b/src/applications/auth/provider/PhabricatorAuthProvider.php
@@ -464,12 +464,14 @@
public function getAuthCSRFCode(AphrontRequest $request) {
$phcid = $request->getCookie(PhabricatorCookies::COOKIE_CLIENTID);
if (!strlen($phcid)) {
- throw new Exception(
+ throw new AphrontMalformedRequestException(
+ pht('Missing Client ID Cookie'),
pht(
'Your browser did not submit a "%s" cookie with client state '.
'information in the request. Check that cookies are enabled. '.
'If this problem persists, you may need to clear your cookies.',
- PhabricatorCookies::COOKIE_CLIENTID));
+ PhabricatorCookies::COOKIE_CLIENTID),
+ true);
}
return PhabricatorHash::digest($phcid);

File Metadata

Mime Type
text/plain
Expires
Mon, Nov 11, 7:51 PM (1 w, 11 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6715652
Default Alt Text
D16402.id39444.diff (6 KB)

Event Timeline

Phabricator · Built by Phacility