Refactoring exception handling so codes are much more flexible and easy to change.

Made error404 and error500 more generic.
Removed error() as it didn't really make that much sense.
This commit is contained in:
Mark Story 2010-09-04 15:38:10 -04:00
parent dbd34c75c7
commit 4d863618f5
2 changed files with 43 additions and 52 deletions

View file

@ -100,22 +100,27 @@ class ErrorHandler {
return $this->controller->appError($exception);
}
$method = $template = Inflector::variable(str_replace('Exception', '', get_class($exception)));
$code = $exception->getCode();
if ($exception instanceof CakeException && !in_array($method, get_class_methods($this))) {
$methodExists = method_exists($this, $method);
if ($exception instanceof CakeException && !$methodExists) {
$method = '_cakeError';
} elseif (!method_exists($this, $method)) {
} elseif (!$methodExists) {
$method = 'error500';
if ($code >= 400) {
$method = 'error400';
}
}
if ($method !== 'error' && Configure::read('debug') == 0) {
$code = $exception->getCode();
if (Configure::read('debug') == 0) {
$parentClass = get_parent_class($this);
if ($parentClass != 'ErrorHandler') {
$method = 'error404';
$method = 'error400';
}
$parentMethods = (array)get_class_methods($parentClass);
if (in_array($method, $parentMethods)) {
$method = 'error404';
$method = 'error400';
}
if ($code == 500) {
$method = 'error500';
@ -179,15 +184,6 @@ class ErrorHandler {
call_user_func_array(array($this, $this->method), array($this->error));
}
/**
* Displays an error page (e.g. 404 Not found).
*
* @param array $params Parameters for controller
*/
public function error(Exception $error) {
$this->error404($error);
}
/**
* Generic handler for the internal framework errors CakePHP can generate.
*
@ -207,23 +203,22 @@ class ErrorHandler {
}
/**
* Convenience method to display a 404 page.
* Convenience method to display a 400 series page.
*
* @param array $params Parameters for controller
*/
public function error404($error) {
public function error400($error) {
$message = $error->getMessage();
if (Configure::read('debug') == 0 && $error instanceof CakeException) {
$message = __('Not Found');
}
$url = Router::normalize($this->controller->request->here);
$this->controller->response->statusCode(404);
$this->controller->response->statusCode($error->getCode());
$this->controller->set(array(
'code' => 404,
'name' => $message,
'url' => h($url),
));
$this->_outputMessage('error404');
$this->_outputMessage('error400');
}
/**
@ -231,9 +226,9 @@ class ErrorHandler {
*
* @param array $params Parameters for controller
*/
public function error500($params) {
public function error500($error) {
$url = Router::normalize($this->controller->request->here);
$this->controller->response->statusCode(500);
$this->controller->response->statusCode($error->getCode());
$this->controller->set(array(
'name' => __('An Internal Error Has Occurred'),
'message' => h($url),

View file

@ -248,7 +248,7 @@ class ErrorHandlerTest extends CakeTestCase {
/**
* test that methods declared in an ErrorHandler subclass are not converted
* into error404 when debug > 0
* into error400 when debug > 0
*
* @return void
*/
@ -281,7 +281,7 @@ class ErrorHandlerTest extends CakeTestCase {
$ErrorHandler->render();
$result = ob_get_clean();
$this->assertEqual($result, 'widget thing is missing', 'Method declared in subclass converted to error404');
$this->assertEqual($result, 'widget thing is missing', 'Method declared in subclass converted to error400');
}
/**
@ -295,13 +295,13 @@ class ErrorHandlerTest extends CakeTestCase {
$exception = new MissingControllerException('PostsController');
$ErrorHandler = new MyCustomErrorHandler($exception);
$this->assertEqual('error404', $ErrorHandler->method);
$this->assertEqual('error400', $ErrorHandler->method);
ob_start();
$ErrorHandler->render();
$result = ob_get_clean();
$this->assertPattern('/Not Found/', $result, 'Method declared in error handler not converted to error404. %s');
$this->assertPattern('/Not Found/', $result, 'Method declared in error handler not converted to error400. %s');
}
/**
@ -314,7 +314,7 @@ class ErrorHandlerTest extends CakeTestCase {
$ErrorHandler = new ErrorHandler($exception);
$this->assertType('CakeErrorController', $ErrorHandler->controller);
$this->assertEquals('error404', $ErrorHandler->method);
$this->assertEquals('error400', $ErrorHandler->method);
$this->assertEquals($exception, $ErrorHandler->error);
}
@ -329,46 +329,42 @@ class ErrorHandlerTest extends CakeTestCase {
$ErrorHandler = new ErrorHandler($exception);
$this->assertType('CakeErrorController', $ErrorHandler->controller);
$this->assertEquals('error404', $ErrorHandler->method);
$this->assertEquals('error400', $ErrorHandler->method);
$this->assertEquals($exception, $ErrorHandler->error);
}
/**
* test that unknown exception types are captured and converted to 500
* test that unknown exception types with valid status codes are treated correctly.
*
* @return void
*/
function testUnknownExceptionType() {
function testUnknownExceptionTypeWithExceptionThatHasA400Code() {
$exception = new MissingWidgetThingException('coding fail.');
$ErrorHandler = new ErrorHandler($exception);
$this->assertFalse(method_exists($ErrorHandler, 'missingWidgetThing'), 'no method should exist.');
$this->assertEquals('error400', $ErrorHandler->method, 'incorrect method coercion.');
}
/**
* test that unknown exception types with valid status codes are treated correctly.
*
* @return void
*/
function testUnknownExceptionTypeWithNoCodeIsA500() {
$exception = new OutOfBoundsException('foul ball.');
$ErrorHandler = new ErrorHandler($exception);
$this->assertEquals('error500', $ErrorHandler->method, 'incorrect method coercion.');
}
/**
* testError method
* testerror400 method
*
* @access public
* @return void
*/
function testError() {
$exception = new Exception('Page not found');
$ErrorHandler = new ErrorHandler($exception);
ob_start();
$ErrorHandler->error($exception);
$result = ob_get_clean();
$this->assertPattern("/<h2>Page not found<\/h2>/", $result);
}
/**
* testError404 method
*
* @access public
* @return void
*/
function testError404() {
function testerror400() {
App::build(array(
'views' => array(TEST_CAKE_CORE_INCLUDE_PATH . 'libs' . DS . 'view' . DS)
), true);
@ -393,11 +389,11 @@ class ErrorHandlerTest extends CakeTestCase {
}
/**
* test that error404 only modifies the messages on CakeExceptions.
* test that error400 only modifies the messages on CakeExceptions.
*
* @return void
*/
function testError404OnlyChangingCakeException() {
function testerror400OnlyChangingCakeException() {
Configure::write('debug', 0);
$exception = new Error404Exception('Custom message');
@ -417,11 +413,11 @@ class ErrorHandlerTest extends CakeTestCase {
$this->assertContains('Not Found', $result);
}
/**
* test that error404 doesn't expose XSS
* test that error400 doesn't expose XSS
*
* @return void
*/
function testError404NoInjection() {
function testerror400NoInjection() {
Router::reload();
$request = new CakeRequest('pages/<span id=333>pink</span></id><script>document.body.style.background = t=document.getElementById(333).innerHTML;window.alert(t);</script>', false);