MINOR: Updated Controller to return a 404 on actions that don't exist, rather than a 403.

From: Andrew Short <andrewjshort@gmail.com>

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@88506 467b73ca-7a2a-4603-9d3b-597d59a354a9
This commit is contained in:
Andrew Short 2009-10-11 00:07:24 +00:00 committed by Sam Minnee
parent da4b65c749
commit 53450e1221
3 changed files with 21 additions and 8 deletions

View File

@ -183,6 +183,10 @@ class Controller extends RequestHandler {
$this->requestParams = $request->requestVars();
if(!$this->action) $this->action = 'index';
if(!$this->hasAction($this->action)) {
$this->httpError(404, "The action '$this->action' does not exist in class $this->class");
}
// 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'))) {
return $this->httpError(403, "Action '$this->action' isn't allowed on class $this->class");
@ -198,11 +202,7 @@ class Controller extends RequestHandler {
return $result;
}
} else {
if($this->action == 'index' || $this->hasAction($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");
}
return $this->getViewer($this->action)->process($this);
}
}

View File

@ -38,7 +38,7 @@ class ControllerTest extends FunctionalTest {
$this->assertEquals(200, $response->getStatusCode());
$response = $this->get("ControllerTest_SecuredController/stringaction");
$this->assertEquals(403, $response->getStatusCode());
$this->assertEquals(404, $response->getStatusCode());
$response = $this->get("ControllerTest_SecuredController/adminonly");
$this->assertEquals(403, $response->getStatusCode());
@ -113,6 +113,13 @@ class ControllerTest extends FunctionalTest {
$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');
$unsecured = new ControllerTest_HasAction_Unsecured();
$this->assertTrue (
$unsecured->hasAction('defined_action'),
'Without an allowed_actions, any defined methods are recognised as actions'
);
}
}
@ -189,3 +196,9 @@ class ControllerTest_HasAction extends Controller {
);
}
class ControllerTest_HasAction_Unsecured extends ControllerTest_HasAction {
public function defined_action() { }
}

4
tests/RequestHandlingTest.php Normal file → Executable file
View File

@ -86,7 +86,7 @@ class RequestHandlingTest extends SapphireTest {
function testDisallowedExtendedActions() {
/* Actions on magic methods are only accessible if explicitly allowed on the controller. */
$response = Director::test("testGoodBase1/extendedMethod");
$this->assertEquals(403, $response->getStatusCode());
$this->assertEquals(404, $response->getStatusCode());
/* Actions on an extension are allowed because they specifically provided appropriate allowed_actions items */
$response = Director::test("testGoodBase1/otherExtendedMethod");
@ -94,7 +94,7 @@ class RequestHandlingTest extends SapphireTest {
/* The failoverMethod action wasn't explicitly listed and so isnt' allowed */
$response = Director::test("testGoodBase1/failoverMethod");
$this->assertEquals(403, $response->getStatusCode());
$this->assertEquals(404, $response->getStatusCode());
/* However, on RequestHandlingTest_AllowedController it has been explicitly allowed */
$response = Director::test("RequestHandlingTest_AllowedController/failoverMethod");