From eef3ff80212b9fa12de76eff3ebbb7ba59e51d9e Mon Sep 17 00:00:00 2001
From: Sam Minnee <sam@silverstripe.com>
Date: Wed, 13 Oct 2010 03:30:54 +0000
Subject: [PATCH] BUGFIX: disallow numeric actions - numeric array indexes are
 incorrectly picked up as allowed actions (#5331) (from r103092)

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@112118 467b73ca-7a2a-4603-9d3b-597d59a354a9
---
 core/control/RequestHandler.php | 11 ++++++++---
 tests/ControllerTest.php        |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/core/control/RequestHandler.php b/core/control/RequestHandler.php
index 551f23286..10f4c3427 100755
--- a/core/control/RequestHandler.php
+++ b/core/control/RequestHandler.php
@@ -216,10 +216,15 @@ class RequestHandler extends ViewableData {
 		$action  = strtolower($action);
 		$actions = $this->allowedActions();
 		
+		// Check if the action is defined in the allowed actions as either a
+		// key or value. Note that if the action is numeric, then keys are not
+		// searched for actions to prevent actual array keys being recognised
+		// as actions.
 		if(is_array($actions)) {
-			if(array_key_exists($action, $actions) || in_array($action, $actions)) {
-				return true;
-			}
+			$isKey   = !is_numeric($action) && array_key_exists($action, $actions);
+			$isValue = in_array($action, $actions);
+
+			if($isKey || $isValue) return true;
 		}
 		
 		if(!is_array($actions) || !$this->uninherited('allowed_actions')) {
diff --git a/tests/ControllerTest.php b/tests/ControllerTest.php
index f2e82b5af..313e0fd04 100755
--- a/tests/ControllerTest.php
+++ b/tests/ControllerTest.php
@@ -110,6 +110,7 @@ class ControllerTest extends FunctionalTest {
 	public function testHasAction() {
 		$controller = new ControllerTest_HasAction();
 		
+		$this->assertFalse($controller->hasAction('1'), 'Numeric actions do not slip through.');
 		$this->assertFalse($controller->hasAction('undefined'), 'undefined actions do not exist');
 		$this->assertTrue($controller->hasAction('allowed_action'), 'allowed actions are recognised');
 		$this->assertTrue($controller->hasAction('template_action'), 'action-specific templates are recognised');