FIX: Removed unnecessary database_is_ready call.

This shaves about 45ms from every request (PHP 7.1 on a 2013 rMBP), 
cutting down execution time of a “hello world” controller by about 33%.

database_is_ready is still used in dev/build and ?flush=1 to stop people
from people bypassing security by DOSing the database or otherwise
forcing a DatabaseException
This commit is contained in:
Sam Minnee 2017-08-25 13:06:12 +12:00
parent 85f4a41c67
commit 8c15e451c6
3 changed files with 30 additions and 24 deletions

View File

@ -7,6 +7,7 @@ use SilverStripe\Control\HTTPResponse;
use SilverStripe\Control\Middleware\HTTPMiddleware; use SilverStripe\Control\Middleware\HTTPMiddleware;
use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Config\Configurable;
use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\ValidationException;
use SilverStripe\ORM\Connect\DatabaseException;
class AuthenticationMiddleware implements HTTPMiddleware class AuthenticationMiddleware implements HTTPMiddleware
{ {
@ -44,17 +45,17 @@ class AuthenticationMiddleware implements HTTPMiddleware
*/ */
public function process(HTTPRequest $request, callable $delegate) public function process(HTTPRequest $request, callable $delegate)
{ {
if (Security::database_is_ready()) { try {
try { $this
$this ->getAuthenticationHandler()
->getAuthenticationHandler() ->authenticateRequest($request);
->authenticateRequest($request); } catch (ValidationException $e) {
} catch (ValidationException $e) { return new HTTPResponse(
return new HTTPResponse( "Bad log-in details: " . $e->getMessage(),
"Bad log-in details: " . $e->getMessage(), 400
400 );
); } catch (DatabaseException $e) {
} // Database isn't ready, carry on.
} }
return $delegate($request); return $delegate($request);

View File

@ -10,6 +10,7 @@ use SilverStripe\Control\HTTPResponse_Exception;
use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Config\Configurable;
use SilverStripe\Dev\Debug; use SilverStripe\Dev\Debug;
use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator; use SilverStripe\Security\MemberAuthenticator\MemberAuthenticator;
use SilverStripe\ORM\Connect\DatabaseException;
/** /**
* Provides an interface to HTTP basic authentication. * Provides an interface to HTTP basic authentication.
@ -72,7 +73,7 @@ class BasicAuth
$permissionCode = null, $permissionCode = null,
$tryUsingSessionLogin = true $tryUsingSessionLogin = true
) { ) {
if (!Security::database_is_ready() || (Director::is_cli() && static::config()->get('ignore_cli'))) { if ((Director::is_cli() && static::config()->get('ignore_cli'))) {
return true; return true;
} }
@ -94,19 +95,24 @@ class BasicAuth
$member = null; $member = null;
if ($request->getHeader('PHP_AUTH_USER') && $request->getHeader('PHP_AUTH_PW')) { try {
/** @var MemberAuthenticator $authenticator */ if ($request->getHeader('PHP_AUTH_USER') && $request->getHeader('PHP_AUTH_PW')) {
$authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::LOGIN); /** @var MemberAuthenticator $authenticator */
$authenticators = Security::singleton()->getApplicableAuthenticators(Authenticator::LOGIN);
foreach ($authenticators as $name => $authenticator) { foreach ($authenticators as $name => $authenticator) {
$member = $authenticator->authenticate([ $member = $authenticator->authenticate([
'Email' => $request->getHeader('PHP_AUTH_USER'), 'Email' => $request->getHeader('PHP_AUTH_USER'),
'Password' => $request->getHeader('PHP_AUTH_PW'), 'Password' => $request->getHeader('PHP_AUTH_PW'),
], $request); ], $request);
if ($member instanceof Member) { if ($member instanceof Member) {
break; break;
}
} }
} }
} catch (DatabaseException $e) {
// Database isn't ready, let people in
return true;
} }
if ($member instanceof Member) { if ($member instanceof Member) {

View File

@ -107,8 +107,7 @@ class CookieAuthenticationHandler implements AuthenticationHandler
$uidAndToken = Cookie::get($this->getTokenCookieName()); $uidAndToken = Cookie::get($this->getTokenCookieName());
$deviceID = Cookie::get($this->getDeviceCookieName()); $deviceID = Cookie::get($this->getDeviceCookieName());
// @todo Consider better placement of database_is_ready test if ($deviceID === null || strpos($uidAndToken, ':') === false) {
if ($deviceID === null || strpos($uidAndToken, ':') === false || !Security::database_is_ready()) {
return null; return null;
} }