Merge pull request #827 from sminnee/http-error-catching

API: Added 'onBeforeHTTPError' and 'onBeforeHTTPError<code>' extension p...

control/HTTPResponse.php

@@ -293,13 +293,25 @@ class SS_HTTPResponse_Exception extends Exception {
 	protected $response;
 	
 	/**
+	 * @param  string|SS_HTTPResponse body Either the plaintext content of the error message, or an SS_HTTPResponse
+	 *                                     object representing it.  In either case, the $statusCode and
+	 *                                     $statusDescription will be the HTTP status of the resulting response.
 	 * @see SS_HTTPResponse::__construct();
 	 */
 	 public function __construct($body = null, $statusCode = null, $statusDescription = null) {
 	 	if($body instanceof SS_HTTPResponse) {
-	 		$this->setResponse($body);
+	 		// statusCode and statusDescription should override whatever is passed in the body
+	 		if($statusCode) $body->setStatusCode($statusCode);
+			if($statusDescription) $body->setStatusDescription($statusDescription);
+
+			$this->setResponse($body);
 	 	} else {
-	 		$this->setResponse(new SS_HTTPResponse($body, $statusCode, $statusDescription));
+			$response = new SS_HTTPResponse($body, $statusCode, $statusDescription);
+
+			// Error responses should always be considered plaintext, for security reasons
+			$response->addHeader('Content-Type', 'text/plain');
+
+	 		$this->setResponse($response);
 	 	}
 	 	
 	 	parent::__construct($this->getResponse()->getBody(), $this->getResponse()->getStatusCode());

control/RequestHandler.php

@@ -332,12 +332,14 @@ class RequestHandler extends ViewableData {
 	 * @uses SS_HTTPResponse_Exception
 	 */
 	public function httpError($errorCode, $errorMessage = null) {
-		$e = new SS_HTTPResponse_Exception($errorMessage, $errorCode);
+		// Call a handler method such as onBeforeHTTPError404
+		$this->extend('onBeforeHTTPError' . $errorCode, $this->request);
 
-		// Error responses should always be considered plaintext, for security reasons
+		// Call a handler method such as onBeforeHTTPError, passing 404 as the first arg
-		$e->getResponse()->addHeader('Content-Type', 'text/plain');
+		$this->extend('onBeforeHTTPError', $errorCode, $this->request);
 
-		throw $e;
+		// Throw a new exception
+		throw new SS_HTTPResponse_Exception($errorMessage, $errorCode);
 	}
 
 	/**

tests/control/HTTPResponseTest.php

@@ -30,4 +30,35 @@ class HTTPResponseTest extends SapphireTest {
 		);
 	}
 	
+	public function testHTTPResponseException() {
+		$response = new SS_HTTPResponse("Test", 200, 'OK');
+
+		// Confirm that the exception's statusCode and statusDescription take precedence
+		try {
+			throw new SS_HTTPResponse_Exception($response, 404, 'not even found');
+
+		} catch(SS_HTTPResponse_Exception $e) {
+			$this->assertEquals(404, $e->getResponse()->getStatusCode());
+			$this->assertEquals('not even found', $e->getResponse()->getStatusDescription());
+			return;
+		}
+		// Fail if we get to here
+		$this->assertFalse(true, 'Something went wrong with our test exception');
+
+	}
+
+	public function testExceptionContentPlainByDefault() {
+
+		// Confirm that the exception's statusCode and statusDescription take precedence
+		try {
+			throw new SS_HTTPResponse_Exception("Some content that may be from a hacker", 404, 'not even found');
+
+		} catch(SS_HTTPResponse_Exception $e) {
+			$this->assertEquals("text/plain", $e->getResponse()->getHeader("Content-Type"));
+			return;
+		}
+		// Fail if we get to here
+		$this->assertFalse(true, 'Something went wrong with our test exception');
+
+	}
 }

tests/control/RequestHandlingTest.php

@@ -146,9 +146,17 @@ class RequestHandlingTest extends FunctionalTest {
 	}
 	
 	public function testHTTPError() {
+		RequestHandlingTest_ControllerExtension::$called_error = false;
+		RequestHandlingTest_ControllerExtension::$called_404_error = false;
+
 		$response = Director::test('RequestHandlingTest_Controller/throwhttperror');
 		$this->assertEquals(404, $response->getStatusCode());
 		$this->assertEquals('This page does not exist.', $response->getBody());
+
+		// Confirm that RequestHandlingTest_ControllerExtension::onBeforeHTTPError() called
+		$this->assertTrue(RequestHandlingTest_ControllerExtension::$called_error);
+		// Confirm that RequestHandlingTest_ControllerExtension::onBeforeHTTPError404() called
+		$this->assertTrue(RequestHandlingTest_ControllerExtension::$called_404_error);
 	}
 	
 	public function testMethodsOnParentClassesOfRequestHandlerDeclined() {
@@ -401,9 +409,27 @@ class RequestHandlingTest_FormActionController extends Controller {
  * Simple extension for the test controller
  */
 class RequestHandlingTest_ControllerExtension extends Extension {
+	public static $called_error = false;
+	public static $called_404_error = false;
+
 	public function extendedMethod() {
 		return "extendedMethod";
 	}
+
+	/**
+	 * Called whenever there is an HTTP error
+	 */
+	public function onBeforeHTTPError() {
+		self::$called_error = true;
+	}
+
+	/**
+	 * Called whenever there is an 404 error
+	 */
+	public function onBeforeHTTPError404() {
+		self::$called_404_error = true;
+	}
+
 }
 
 /**