ENHANCEMENT: Do not allow access to actions that have not been defined or do not have an action template.

ENHANCEMENT: Added Controller->hasActionTemplate() to check if a template exists for a specific action.

From: Andrew Short <andrewjshort@gmail.com>

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@88477 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/control/Controller.php

@@ -173,7 +173,7 @@ class Controller extends RequestHandler {
 	 * Controller's default action handler.  It will call the method named in $Action, if that method exists.
 	 * If $Action isn't given, it will use "index" as a default.
 	 */
-	function handleAction($request) {
+	public function handleAction($request) {
 		// urlParams, requestParams, and action are set for backward compatability 
 		foreach($request->latestParams() as $k => $v) {
 			if($v || !isset($this->urlParams[$k])) $this->urlParams[$k] = $v;
@@ -182,28 +182,27 @@ class Controller extends RequestHandler {
 		$this->action = str_replace("-","_",$request->param('Action'));
 		$this->requestParams = $request->requestVars();
 		if(!$this->action) $this->action = 'index';
-		$methodName = $this->action;
 		
 		// run & init are manually disabled, because they create infinite loops and other dodgy situations 
-		if($this->checkAccessAction($this->action) && !in_array(strtolower($this->action), array('run', 'init'))) {
-			if($this->hasMethod($methodName)) {
-				$result = $this->$methodName($request);
+		if(!$this->checkAccessAction($this->action) || in_array(strtolower($this->action), array('run', 'init'))) {
+			return $this->httpError(403, "Action '$this->action' isn't allowed on class $this->class");
+		}
 		
-				// Method returns an array, that is used to customise the object before rendering with a template
-				if(is_array($result)) {
-					return $this->getViewer($this->action)->process($this->customise($result));
+		if($this->hasMethod($this->action)) {
+			$result = $this->{$this->action}($request);
 			
-				// Method returns a string / object, in which case we just return that
-				} else {
-					return $result;
-				}
-			
-			// There is no method, in which case we just render this object using a (possibly alternate) template
+			// If the action returns an array, customise with it before rendering the template.
+			if(is_array($result)) {
+				return $this->getViewer($this->action)->process($this->customise($result));
 			} else {
-				return $this->getViewer($this->action)->process($this);
+				return $result;
 			}
 		} else {
-			return $this->httpError(403, "Action '$this->action' isn't allowed on class $this->class");
+			if($this->action == 'index' || $this->hasActionTemplate($this->action)) {
+				return $this->getViewer($this->action)->process($this);
+			} else {
+				return $this->httpError(404, "The action '$this->action' does not exist in class $this->class");
+			}
 		}
 	}
 
@@ -307,6 +306,28 @@ class Controller extends RequestHandler {
 		return new SSViewer($templates);
 	}
 
+	/**
+	 * Returns TRUE if this controller has a template that is specifically designed to handle a specific action.
+	 *
+	 * @param string $action
+	 * @return bool
+	 */
+	public function hasActionTemplate($action = null) {
+		if(!$action) $action = $this->action;
+		
+		if(isset($this->templates[$action])) return true;
+		
+		$parentClass = $this->class;
+		$templates   = array();
+		
+		while($parentClass != 'Controller') {
+			$templates[] = strtok($parentClass, '_') . '_' . $action;
+			$parentClass = get_parent_class($parentClass);
+		}
+		
+		return SSViewer::hasTemplate($templates);
+	}
+	
 	/**
 	 * Render the current controller with the templates determined
 	 * by {@link getViewer()}.

tests/ControllerTest.php

@@ -26,6 +26,11 @@ class ControllerTest extends FunctionalTest {
 		$this->assertRegExp("/This is the template for templateaction. Content is 'default content'./", $response->getBody());
 	}
 	
+	public function testUndefinedActions() {
+		$response = Director::test('ControllerTest_UnsecuredController/undefinedaction');
+		$this->assertEquals(404, $response->getStatusCode(), 'Undefined actions return a not found response.');
+	}
+	
 	function testAllowedActions() {
 		$adminUser = $this->objFromFixture('Member', 'admin');