Skip to content
Permalink
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...
markstory committed Feb 11, 2012
1 parent 0207a61 commit 6f914174a63c2af27547d5123409d36865784abe
@@ -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);

This comment has been minimized.

Copy link
@0x20h

0x20h Feb 12, 2012

Contributor

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

This comment has been minimized.

Copy link
@markstory

markstory Feb 12, 2012

Author Member

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

This comment has been minimized.

Copy link
@lorenzo

lorenzo Feb 12, 2012

Member

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?

This comment has been minimized.

Copy link
@markstory

markstory Feb 12, 2012

Author Member

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

This comment has been minimized.

Copy link
@ceeram

ceeram Feb 12, 2012

Member

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

This comment has been minimized.

Copy link
@0x20h

0x20h Feb 12, 2012

Contributor

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.

return trim($path, '/');
}
/**
@@ -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.
You can’t perform that action at this time.