Skip to content
Browse files

Fix issues with double / & leading/trailing /

Authorize classes should remove // and leading trailing /
Without this incorrect paths that fail to match nodes can be
generated.  This also allows settings[actionPath] to be
permissive in what it accepts.

Fixes #2563
  • Loading branch information...
1 parent 0207a61 commit 6f914174a63c2af27547d5123409d36865784abe @markstory markstory committed Feb 11, 2012
View
4 lib/Cake/Controller/Component/Auth/BaseAuthorize.php
@@ -108,11 +108,13 @@ public function controller($controller = null) {
*/
public function action($request, $path = '/:plugin/:controller/:action') {
$plugin = empty($request['plugin']) ? null : Inflector::camelize($request['plugin']) . '/';
- return str_replace(
+ $path = str_replace(
array(':controller', ':action', ':plugin/'),
array(Inflector::camelize($request['controller']), $request['action'], $plugin),
$this->settings['actionPath'] . $path
);
+ $path = str_replace('//', '/', $path);
@0x20h
0x20h added a note Feb 12, 2012

just to make extra sure, probably better to preg_replace('#/+#', '/', $path)

@markstory
CakePHP member
markstory added a note Feb 12, 2012

I don't know how you'd end up with 3 slashes though.

@lorenzo
CakePHP member
lorenzo added a note Feb 12, 2012

I think this was changed by Ceeram not long ago to fix a bug in ACL, he made sure that there was a slash at the beginning of every path. What is the correct way?

@markstory
CakePHP member
markstory added a note Feb 12, 2012

In the comments on lighthouse, I thought @ceeram mentioned that the leading slash was a problem. Ticket was http://cakephp.lighthouseapp.com/projects/42648/tickets/2563

@ceeram
CakePHP member
ceeram added a note Feb 12, 2012

i did not change this afaik, the leading slash indeed causes issues, i discussed that a while back already in irc, it is fine as it is now

@0x20h
0x20h added a note Feb 12, 2012

me too @markstory. But I thought as $path is a parameter it would probably make sense to be a bit more fault-tolerant. But, of course, it's ok as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return trim($path, '/');
}
/**
View
25 lib/Cake/Test/Case/Controller/Component/Auth/ActionsAuthorizeTest.php
@@ -75,7 +75,7 @@ public function testAuthorizeFailure() {
$this->Acl->expects($this->once())
->method('check')
- ->with($user, '/controllers/Posts/index')
+ ->with($user, 'controllers/Posts/index')
->will($this->returnValue(false));
$this->assertFalse($this->auth->authorize($user['User'], $request));
@@ -104,7 +104,7 @@ public function testAuthorizeSuccess() {
$this->Acl->expects($this->once())
->method('check')
- ->with($user, '/controllers/Posts/index')
+ ->with($user, 'controllers/Posts/index')
->will($this->returnValue(true));
$this->assertTrue($this->auth->authorize($user['User'], $request));
@@ -134,7 +134,7 @@ public function testAuthorizeSettings() {
$expected = array('TestPlugin.TestPluginAuthUser' => array('id' => 1, 'user' => 'mariano'));
$this->Acl->expects($this->once())
->method('check')
- ->with($expected, '/controllers/Posts/index')
+ ->with($expected, 'controllers/Posts/index')
->will($this->returnValue(true));
$this->assertTrue($this->auth->authorize($user, $request));
@@ -154,8 +154,23 @@ public function testActionMethod() {
));
$result = $this->auth->action($request);
+ $this->assertEquals('controllers/Posts/index', $result);
+ }
- $this->assertEquals('/controllers/Posts/index', $result);
+/**
+ * Make sure that action() doesn't create double slashes anywhere.
+ *
+ * @return void
+ */
+ public function testActionNoDoubleSlash() {
+ $this->auth->settings['actionPath'] = '/controllers/';
+ $request = array(
+ 'plugin' => null,
+ 'controller' => 'posts',
+ 'action' => 'index'
+ );
+ $result = $this->auth->action($request);
+ $this->assertEquals('controllers/Posts/index', $result);
}
/**
@@ -172,6 +187,6 @@ public function testActionWithPlugin() {
));
$result = $this->auth->action($request);
- $this->assertEquals('/controllers/DebugKit/Posts/index', $result);
+ $this->assertEquals('controllers/DebugKit/Posts/index', $result);
}
}

0 comments on commit 6f91417

Please sign in to comment.
Something went wrong with that request. Please try again.