From cf2cf11b57a76a2e6fb2879a3ae30553b6e0912c Mon Sep 17 00:00:00 2001
From: Andrew Short <andrewjshort@gmail.com>
Date: Sun, 11 Oct 2009 00:07:01 +0000
Subject: [PATCH] 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 | 57 +++++++++++++++++++++++++------------
 tests/ControllerTest.php    |  7 ++++-
 2 files changed, 45 insertions(+), 19 deletions(-)
 mode change 100644 => 100755 core/control/Controller.php

diff --git a/core/control/Controller.php b/core/control/Controller.php
old mode 100644
new mode 100755
index d5baaf78d..ae0e9865f
--- a/core/control/Controller.php
+++ b/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");
+		}
+		
+		if($this->hasMethod($this->action)) {
+			$result = $this->{$this->action}($request);
 			
-				// 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));
-				
-				// 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");
+			}
 		}
 	}
 
@@ -306,6 +305,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
diff --git a/tests/ControllerTest.php b/tests/ControllerTest.php
index 1e66139a9..f14cb7b2f 100644
--- a/tests/ControllerTest.php
+++ b/tests/ControllerTest.php
@@ -25,7 +25,12 @@ class ControllerTest extends FunctionalTest {
 		$response = $this->get("ControllerTest_Controller/templateaction");
 		$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');