Browse Source

Fix incorrect path indexing for greedy routes.

Greedy routes should be indexed without the trailing /. Failing to do
that makes paths without trailing arguments fail.

Fixes #3943
mark_story 11 years ago
parent
commit
20a8f2d991

+ 2 - 1
src/Routing/Route/Route.php

@@ -588,7 +588,8 @@ class Route {
 		}
 		$star = strpos($this->template, '*');
 		if ($star !== false) {
-			return substr($this->template, 0, $star);
+			$path = rtrim(substr($this->template, 0, $star), '/');
+			return $path === '' ? '/' : $path;
 		}
 		return $this->template;
 	}

+ 18 - 1
tests/TestCase/Routing/Route/RouteTest.php

@@ -582,6 +582,20 @@ class RouteTest extends TestCase {
 		$result = $route->parse('/admin/posts');
 		$this->assertEquals('posts', $result['controller']);
 		$this->assertEquals('index', $result['action']);
+
+		$route = new Route(
+			'/media/search/*',
+			['controller' => 'Media', 'action' => 'search']
+		);
+		$result = $route->parse('/media/search');
+		$this->assertEquals('Media', $result['controller']);
+		$this->assertEquals('search', $result['action']);
+		$this->assertEquals([], $result['pass']);
+
+		$result = $route->parse('/media/search/tv/shows');
+		$this->assertEquals('Media', $result['controller']);
+		$this->assertEquals('search', $result['action']);
+		$this->assertEquals(['tv', 'shows'], $result['pass']);
 	}
 
 /**
@@ -941,8 +955,11 @@ class RouteTest extends TestCase {
  * @return void
  */
 	public function testStaticPath() {
+		$route = new Route('/*', ['controller' => 'Pages', 'action' => 'display']);
+		$this->assertEquals('/', $route->staticPath());
+
 		$route = new Route('/pages/*', ['controller' => 'Pages', 'action' => 'display']);
-		$this->assertEquals('/pages/', $route->staticPath());
+		$this->assertEquals('/pages', $route->staticPath());
 
 		$route = new Route('/pages/:id/*', ['controller' => 'Pages', 'action' => 'display']);
 		$this->assertEquals('/pages/', $route->staticPath());

+ 21 - 0
tests/TestCase/Routing/RouteCollectionTest.php

@@ -56,6 +56,7 @@ class RouteCollectionTest extends TestCase {
 		$routes = new RouteBuilder($this->collection, '/b', ['key' => 'value']);
 		$routes->connect('/', ['controller' => 'Articles']);
 		$routes->connect('/:id', ['controller' => 'Articles', 'action' => 'view']);
+		$routes->connect('/media/search/*', ['controller' => 'Media', 'action' => 'search']);
 
 		$result = $this->collection->parse('/b/');
 		$expected = [
@@ -78,6 +79,26 @@ class RouteCollectionTest extends TestCase {
 			'?' => ['one' => 'two'],
 		];
 		$this->assertEquals($expected, $result);
+
+		$result = $this->collection->parse('/b/media/search');
+		$expected = [
+			'key' => 'value',
+			'pass' => [],
+			'plugin' => null,
+			'controller' => 'Media',
+			'action' => 'search'
+		];
+		$this->assertEquals($expected, $result);
+
+		$result = $this->collection->parse('/b/media/search/thing');
+		$expected = [
+			'key' => 'value',
+			'pass' => ['thing'],
+			'plugin' => null,
+			'controller' => 'Media',
+			'action' => 'search'
+		];
+		$this->assertEquals($expected, $result);
 	}
 
 /**

+ 0 - 1
tests/TestCase/Routing/RouterTest.php

@@ -1990,7 +1990,6 @@ class RouterTest extends TestCase {
 		Router::reload();
 		$adminParams = array('prefix' => 'admin');
 		Router::connect('/admin/:controller', $adminParams);
-		Router::connect('/admin/:controller/:action', $adminParams);
 		Router::connect('/admin/:controller/:action/*', $adminParams);
 
 		$request = new Request();