Skip to content

Commit

Permalink
Reject controller names that are not camel case.
Browse files Browse the repository at this point in the history
Controller names that are not CamelCase cause a few issues downstream:

1. They don't work on case-sensitive filesystems.
2. They cause Controller::$name and Controller::$modelClass to be wrong.

To help reduce these cryptic and environment sensitive issues we reject
all controller names that start with a lower case letter.

Refs #8091
Refs #8085
  • Loading branch information
markstory committed Jan 24, 2016
1 parent 72db96b commit 8994971
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/Controller/Controller.php
Expand Up @@ -237,7 +237,7 @@ public function __construct(Request $request = null, Response $response = null,
}

if ($this->name === null && isset($request->params['controller'])) {
$this->name = Inflector::camelize($request->params['controller']);
$this->name = $request->params['controller'];
}

if ($this->name === null) {
Expand Down
7 changes: 6 additions & 1 deletion src/Routing/Filter/ControllerFactoryFilter.php
Expand Up @@ -74,7 +74,12 @@ protected function _getController($request, $response)
);
$namespace .= '/' . implode('/', $prefixes);
}
if (strpos($controller, '\\') !== false || strpos($controller, '.') !== false) {
$firstChar = substr($controller, 0, 1);
if (
strpos($controller, '\\') !== false ||
strpos($controller, '.') !== false ||
$firstChar === strtolower($firstChar)
) {
return false;
}
$className = false;
Expand Down
6 changes: 0 additions & 6 deletions tests/TestCase/Controller/ControllerTest.php
Expand Up @@ -359,12 +359,6 @@ public function testConstructSetModelClass()
$this->assertEquals('Posts', $controller->modelClass);
$this->assertInstanceOf('Cake\ORM\Table', $controller->Posts);

$request->params['controller'] = 'posts';
$controller = new \TestApp\Controller\PostsController($request, $response);
$this->assertEquals('Posts', $controller->modelClass);
$this->assertInstanceOf('Cake\ORM\Table', $controller->Posts);
unset($request->params['controller']);

$controller = new \TestApp\Controller\Admin\PostsController($request, $response);
$this->assertEquals('Posts', $controller->modelClass);
$this->assertInstanceOf('Cake\ORM\Table', $controller->Posts);
Expand Down
26 changes: 25 additions & 1 deletion tests/TestCase/Routing/DispatcherTest.php
Expand Up @@ -302,6 +302,30 @@ public function testMissingControllerAbstract()
$this->dispatcher->dispatch($request, $response, ['return' => 1]);
}

/**
* Test that lowercase controller names result in missing controller errors.
*
* In case-insensitive file systems, lowercase controller names will kind of work.
* This causes annoying deployment issues for lots of folks.
*
* @expectedException \Cake\Routing\Exception\MissingControllerException
* @expectedExceptionMessage Controller class somepages could not be found.
* @return void
*/
public function testMissingControllerLowercase()
{
$request = new Request([
'url' => 'pages/home',
'params' => [
'controller' => 'somepages',
'action' => 'display',
'pass' => ['home'],
]
]);
$response = $this->getMock('Cake\Network\Response');
$this->dispatcher->dispatch($request, $response, ['return' => 1]);
}

/**
* testDispatch method
*
Expand Down Expand Up @@ -477,7 +501,7 @@ public function testDispatcherFilter()
$request = new Request([
'url' => '/',
'params' => [
'controller' => 'pages',
'controller' => 'Pages',
'action' => 'display',
'home',
'pass' => []
Expand Down

0 comments on commit 8994971

Please sign in to comment.