Merge pull request #5673 from davidsteinsland/fix_exception_handler_nesting

Fixed nesting when ExceptionRenderer throws exception

lib/Cake/Error/ErrorHandler.php

@@ -95,6 +95,14 @@ App::uses('Router', 'Routing');
  */
 class ErrorHandler {
 
+/**
+ * Whether to give up rendering an exception, if the renderer itself is
+ * throwing exceptions.
+ *
+ * @var bool
+ */
+	protected static $_bailExceptionRendering = false;
+
 /**
  * Set as the default exception handler by the CakePHP bootstrap process.
  *
@@ -125,6 +133,8 @@ class ErrorHandler {
 				$e->getMessage(),
 				$e->getTraceAsString()
 			);
+
+			self::$_bailExceptionRendering = true;
 			trigger_error($message, E_USER_ERROR);
 		}
 	}
@@ -234,6 +244,8 @@ class ErrorHandler {
  * @param string $file File on which error occurred
  * @param int $line Line that triggered the error
  * @return bool
+ * @throws FatalErrorException If the Exception renderer threw an exception during rendering, and debug > 0.
+ * @throws InternalErrorException If the Exception renderer threw an exception during rendering, and debug is 0.
  */
 	public static function handleFatalError($code, $description, $file, $line) {
 		$logMessage = 'Fatal Error (' . $code . '): ' . $description . ' in [' . $file . ', line ' . $line . ']';
@@ -249,10 +261,18 @@ class ErrorHandler {
 		}
 
 		if (Configure::read('debug')) {
-			call_user_func($exceptionHandler, new FatalErrorException($description, 500, $file, $line));
+			$exception = new FatalErrorException($description, 500, $file, $line);
 		} else {
-			call_user_func($exceptionHandler, new InternalErrorException());
+			$exception = new InternalErrorException();
 		}
+
+		if (self::$_bailExceptionRendering) {
+			self::$_bailExceptionRendering = false;
+			throw $exception;
+		}
+
+		call_user_func($exceptionHandler, $exception);
+
 		return false;
 	}
 

lib/Cake/Test/Case/Error/ErrorHandlerTest.php

@@ -20,6 +20,23 @@ App::uses('ErrorHandler', 'Error');
 App::uses('Controller', 'Controller');
 App::uses('Router', 'Routing');
 
+/**
+ * A faulty ExceptionRenderer to test nesting.
+ */
+class FaultyExceptionRenderer extends ExceptionRenderer {
+
+/**
+ * Dummy rendering implementation.
+ *
+ * @return void
+ * @throws Exception
+ */
+	public function render() {
+		throw new Exception('Error from renderer.');
+	}
+
+}
+
 /**
  * ErrorHandlerTest class
  *
@@ -320,4 +337,48 @@ class ErrorHandlerTest extends CakeTestCase {
 		$this->assertContains('[FatalErrorException] Something wrong', $log[1], 'message missing.');
 	}
 
+/**
+ * testExceptionRendererNestingDebug method
+ *
+ * @return void
+ */
+	public function testExceptionRendererNestingDebug() {
+		Configure::write('debug', 2);
+		Configure::write('Exception.renderer', 'FaultyExceptionRenderer');
+
+		$result = false;
+		try {
+			ob_start();
+			ob_start();
+			ErrorHandler::handleFatalError(E_USER_ERROR, 'Initial error', __FILE__, __LINE__);
+		} catch (Exception $e) {
+			$result = $e instanceof FatalErrorException;
+		}
+
+		restore_error_handler();
+		$this->assertTrue($result);
+	}
+
+/**
+ * testExceptionRendererNestingProduction method
+ *
+ * @return void
+ */
+	public function testExceptionRendererNestingProduction() {
+		Configure::write('debug', 0);
+		Configure::write('Exception.renderer', 'FaultyExceptionRenderer');
+
+		$result = false;
+		try {
+			ob_start();
+			ob_start();
+			ErrorHandler::handleFatalError(E_USER_ERROR, 'Initial error', __FILE__, __LINE__);
+		} catch (Exception $e) {
+			$result = $e instanceof InternalErrorException;
+		}
+
+		restore_error_handler();
+		$this->assertTrue($result);
+	}
+
 }