From 31f656b6c1e6b3c908e8ac33c136f5111b4b3d4f Mon Sep 17 00:00:00 2001 From: mark_story Date: Sat, 12 Jul 2014 20:28:32 -0400 Subject: [PATCH] Fix error where routes would be incorrectly parsed. Instead of only trying the first path partition, we should check each matching prefix slice until all path prefixes have been exhausted. Fixes #3954 --- src/Routing/RouteCollection.php | 30 +++++++++---------- .../TestCase/Routing/RouteCollectionTest.php | 22 ++++++++++++++ 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/Routing/RouteCollection.php b/src/Routing/RouteCollection.php index 7ba2c83ab18..912c1b48e7f 100644 --- a/src/Routing/RouteCollection.php +++ b/src/Routing/RouteCollection.php @@ -93,25 +93,25 @@ public function add(Route $route, $options = []) { */ public function parse($url) { foreach (array_keys($this->_paths) as $path) { - if (strpos($url, $path) === 0) { - break; + if (strpos($url, $path) !== 0) { + continue; } - } - $queryParameters = null; - if (strpos($url, '?') !== false) { - list($url, $queryParameters) = explode('?', $url, 2); - parse_str($queryParameters, $queryParameters); - } - foreach ($this->_paths[$path] as $route) { - $r = $route->parse($url); - if ($r === false) { - continue; + $queryParameters = null; + if (strpos($url, '?') !== false) { + list($url, $queryParameters) = explode('?', $url, 2); + parse_str($queryParameters, $queryParameters); } - if ($queryParameters) { - $r['?'] = $queryParameters; + foreach ($this->_paths[$path] as $route) { + $r = $route->parse($url); + if ($r === false) { + continue; + } + if ($queryParameters) { + $r['?'] = $queryParameters; + } + return $r; } - return $r; } throw new MissingRouteException(['url' => $url]); } diff --git a/tests/TestCase/Routing/RouteCollectionTest.php b/tests/TestCase/Routing/RouteCollectionTest.php index 0963ce5cda0..ab1593948e2 100644 --- a/tests/TestCase/Routing/RouteCollectionTest.php +++ b/tests/TestCase/Routing/RouteCollectionTest.php @@ -101,6 +101,28 @@ public function testParse() { $this->assertEquals($expected, $result); } +/** + * Test that parsing checks all the related path scopes. + * + * @return void + */ + public function testParseFallback() { + $routes = new RouteBuilder($this->collection, '/', []); + + $routes->resources('Articles'); + $routes->connect('/:controller', ['action' => 'index'], [], ['routeClass' => 'InflectedRoute']); + $routes->connect('/:controller/:action', [], ['routeClass' => 'InflectedRoute']); + + $result = $this->collection->parse('/articles/add'); + $expected = [ + 'controller' => 'Articles', + 'action' => 'add', + 'plugin' => null, + 'pass' => [] + ]; + $this->assertEquals($expected, $result); + } + /** * Test match() throws an error on unknown routes. *