diff --git a/system/Router/AutoRouterImproved.php b/system/Router/AutoRouterImproved.php index e36b84b61c3d..fab6803cbcce 100644 --- a/system/Router/AutoRouterImproved.php +++ b/system/Router/AutoRouterImproved.php @@ -70,6 +70,29 @@ final class AutoRouterImproved implements AutoRouterInterface */ private string $defaultMethod; + /** + * The URI segments. + */ + private array $segments = []; + + /** + * The position of the Controller in the URI segments. + * Null for the default controller. + */ + private ?int $controllerPos = null; + + /** + * The position of the Method in the URI segments. + * Null for the default method. + */ + private ?int $methodPos = null; + + /** + * The position of the first Parameter in the URI segments. + * Null for the no parameters. + */ + private ?int $paramPos = null; + /** * @param class-string[] $protectedControllers * @param string $defaultController Short classname @@ -108,17 +131,21 @@ private function createSegments(string $uri) * If there is a controller corresponding to the first segment, the search * ends there. The remaining segments are parameters to the controller. * - * @param array $segments URI segments - * * @return bool true if a controller class is found. */ - private function searchFirstController(array $segments): bool + private function searchFirstController(): bool { + $segments = $this->segments; + $controller = '\\' . $this->namespace; + $controllerPos = -1; + while ($segments !== []) { $segment = array_shift($segments); - $class = $this->translateURIDashes(ucfirst($segment)); + $controllerPos++; + + $class = $this->translateURIDashes(ucfirst($segment)); // as soon as we encounter any segment that is not PSR-4 compliant, stop searching if (! $this->isValidSegment($class)) { @@ -128,9 +155,14 @@ private function searchFirstController(array $segments): bool $controller .= '\\' . $class; if (class_exists($controller)) { - $this->controller = $controller; + $this->controller = $controller; + $this->controllerPos = $controllerPos; + // The first item may be a method name. $this->params = $segments; + if ($segments !== []) { + $this->paramPos = $this->controllerPos + 1; + } return true; } @@ -142,15 +174,21 @@ private function searchFirstController(array $segments): bool /** * Search for the last default controller corresponding to the URI segments. * - * @param array $segments URI segments - * * @return bool true if a controller class is found. */ - private function searchLastDefaultController(array $segments): bool + private function searchLastDefaultController(): bool { - $params = []; + $segments = $this->segments; + + $segmentCount = count($this->segments); + $paramPos = null; + $params = []; while ($segments !== []) { + if ($segmentCount > count($segments)) { + $paramPos = count($segments); + } + $namespaces = array_map( fn ($segment) => $this->translateURIDashes(ucfirst($segment)), $segments @@ -164,6 +202,10 @@ private function searchLastDefaultController(array $segments): bool $this->controller = $controller; $this->params = $params; + if ($params !== []) { + $this->paramPos = $paramPos; + } + return true; } @@ -179,6 +221,10 @@ private function searchLastDefaultController(array $segments): bool $this->controller = $controller; $this->params = $params; + if ($params !== []) { + $this->paramPos = 0; + } + return true; } @@ -195,19 +241,19 @@ public function getRoute(string $uri, string $httpVerb): array $defaultMethod = strtolower($httpVerb) . ucfirst($this->defaultMethod); $this->method = $defaultMethod; - $segments = $this->createSegments($uri); + $this->segments = $this->createSegments($uri); // Check for Module Routes. if ( - $segments !== [] + $this->segments !== [] && ($routingConfig = config(Routing::class)) - && array_key_exists($segments[0], $routingConfig->moduleRoutes) + && array_key_exists($this->segments[0], $routingConfig->moduleRoutes) ) { - $uriSegment = array_shift($segments); + $uriSegment = array_shift($this->segments); $this->namespace = rtrim($routingConfig->moduleRoutes[$uriSegment], '\\'); } - if ($this->searchFirstController($segments)) { + if ($this->searchFirstController()) { // Controller is found. $baseControllerName = class_basename($this->controller); @@ -219,7 +265,7 @@ public function getRoute(string $uri, string $httpVerb): array 'Cannot access the default controller "' . $this->controller . '" with the controller name URI path.' ); } - } elseif ($this->searchLastDefaultController($segments)) { + } elseif ($this->searchLastDefaultController()) { // The default Controller is found. $baseControllerName = class_basename($this->controller); } else { @@ -227,6 +273,7 @@ public function getRoute(string $uri, string $httpVerb): array throw new PageNotFoundException('No controller is found for: ' . $uri); } + // The first item may be a method name. $params = $this->params; $methodParam = array_shift($params); @@ -241,6 +288,15 @@ public function getRoute(string $uri, string $httpVerb): array $this->method = $method; $this->params = $params; + // Update the positions. + $this->methodPos = $this->paramPos; + if ($params === []) { + $this->paramPos = null; + } + if ($this->paramPos !== null) { + $this->paramPos++; + } + // Prevent access to default controller's method if (strtolower($baseControllerName) === strtolower($this->defaultController)) { throw new PageNotFoundException( @@ -268,6 +324,10 @@ public function getRoute(string $uri, string $httpVerb): array // Ensure the controller does not have _remap() method. $this->checkRemap(); + // Ensure the URI segments for the controller and method do not contain + // underscores when $translateURIDashes is true. + $this->checkUnderscore($uri); + // Check parameter count try { $this->checkParameters($uri); @@ -280,6 +340,20 @@ public function getRoute(string $uri, string $httpVerb): array return [$this->directory, $this->controller, $this->method, $this->params]; } + /** + * @internal For test purpose only. + * + * @return array + */ + public function getPos(): array + { + return [ + 'controller' => $this->controllerPos, + 'method' => $this->methodPos, + 'params' => $this->paramPos, + ]; + } + /** * Get the directory path from the controller and set it to the property. * @@ -363,6 +437,28 @@ private function checkRemap(): void } } + private function checkUnderscore(string $uri): void + { + if ($this->translateURIDashes === false) { + return; + } + + $paramPos = $this->paramPos ?? count($this->segments); + + for ($i = 0; $i < $paramPos; $i++) { + if (strpos($this->segments[$i], '_') !== false) { + throw new PageNotFoundException( + 'AutoRouterImproved prohibits access to the URI' + . ' containing underscores ("' . $this->segments[$i] . '")' + . ' when $translateURIDashes is enabled.' + . ' Please use the dash.' + . ' Handler:' . $this->controller . '::' . $this->method + . ', URI:' . $uri + ); + } + } + } + /** * Returns true if the supplied $segment string represents a valid PSR-4 compliant namespace/directory segment * @@ -373,10 +469,10 @@ private function isValidSegment(string $segment): bool return (bool) preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/', $segment); } - private function translateURIDashes(string $classname): string + private function translateURIDashes(string $segment): string { return $this->translateURIDashes - ? str_replace('-', '_', $classname) - : $classname; + ? str_replace('-', '_', $segment) + : $segment; } } diff --git a/tests/system/Router/AutoRouterImprovedTest.php b/tests/system/Router/AutoRouterImprovedTest.php index affc6a021f51..f18ab64b0ad4 100644 --- a/tests/system/Router/AutoRouterImprovedTest.php +++ b/tests/system/Router/AutoRouterImprovedTest.php @@ -65,6 +65,11 @@ public function testAutoRouteFindsDefaultControllerAndMethodGet() $this->assertSame('\\' . Index::class, $controller); $this->assertSame('getIndex', $method); $this->assertSame([], $params); + $this->assertSame([ + 'controller' => null, + 'method' => null, + 'params' => null, + ], $router->getPos()); } public function testAutoRouteFindsModuleDefaultControllerAndMethodGet() @@ -114,6 +119,11 @@ public function testAutoRouteFindsControllerWithFileAndMethod() $this->assertSame('\\' . Mycontroller::class, $controller); $this->assertSame('getSomemethod', $method); $this->assertSame([], $params); + $this->assertSame([ + 'controller' => 0, + 'method' => 1, + 'params' => null, + ], $router->getPos()); } public function testFindsControllerAndMethodAndParam() @@ -127,6 +137,11 @@ public function testFindsControllerAndMethodAndParam() $this->assertSame('\\' . Mycontroller::class, $controller); $this->assertSame('getSomemethod', $method); $this->assertSame(['a'], $params); + $this->assertSame([ + 'controller' => 0, + 'method' => 1, + 'params' => 2, + ], $router->getPos()); } public function testUriParamCountIsGreaterThanMethodParams() @@ -165,6 +180,11 @@ public function testAutoRouteFindsControllerWithSubfolder() $this->assertSame('\\' . \CodeIgniter\Router\Controllers\Subfolder\Mycontroller::class, $controller); $this->assertSame('getSomemethod', $method); $this->assertSame([], $params); + $this->assertSame([ + 'controller' => 1, + 'method' => 2, + 'params' => null, + ], $router->getPos()); } public function testAutoRouteFindsControllerWithSubSubfolder() @@ -246,6 +266,11 @@ public function testAutoRouteFallbackToDefaultMethod() $this->assertSame('\\' . Index::class, $controller); $this->assertSame('getIndex', $method); $this->assertSame(['15'], $params); + $this->assertSame([ + 'controller' => 0, + 'method' => null, + 'params' => 1, + ], $router->getPos()); } public function testAutoRouteFallbackToDefaultControllerOneParam() @@ -259,6 +284,11 @@ public function testAutoRouteFallbackToDefaultControllerOneParam() $this->assertSame('\\' . \CodeIgniter\Router\Controllers\Subfolder\Home::class, $controller); $this->assertSame('getIndex', $method); $this->assertSame(['15'], $params); + $this->assertSame([ + 'controller' => null, + 'method' => null, + 'params' => 1, + ], $router->getPos()); } public function testAutoRouteFallbackToDefaultControllerTwoParams() @@ -272,6 +302,11 @@ public function testAutoRouteFallbackToDefaultControllerTwoParams() $this->assertSame('\\' . \CodeIgniter\Router\Controllers\Subfolder\Home::class, $controller); $this->assertSame('getIndex', $method); $this->assertSame(['15', '20'], $params); + $this->assertSame([ + 'controller' => null, + 'method' => null, + 'params' => 1, + ], $router->getPos()); } public function testAutoRouteFallbackToDefaultControllerNoParams() @@ -285,6 +320,11 @@ public function testAutoRouteFallbackToDefaultControllerNoParams() $this->assertSame('\\' . \CodeIgniter\Router\Controllers\Subfolder\Home::class, $controller); $this->assertSame('getIndex', $method); $this->assertSame([], $params); + $this->assertSame([ + 'controller' => null, + 'method' => null, + 'params' => null, + ], $router->getPos()); } public function testAutoRouteRejectsSingleDot() @@ -352,4 +392,66 @@ public function testRejectsControllerWithRemapMethod() $router->getRoute('remap/test', 'get'); } + + public function testRejectsURIWithUnderscoreFolder() + { + $this->expectException(PageNotFoundException::class); + $this->expectExceptionMessage( + 'AutoRouterImproved prohibits access to the URI containing underscores ("dash_folder")' + ); + + $router = $this->createNewAutoRouter(); + + $router->getRoute('dash_folder', 'get'); + } + + public function testRejectsURIWithUnderscoreController() + { + $this->expectException(PageNotFoundException::class); + $this->expectExceptionMessage( + 'AutoRouterImproved prohibits access to the URI containing underscores ("dash_controller")' + ); + + $router = $this->createNewAutoRouter(); + + $router->getRoute('dash-folder/dash_controller/dash-method', 'get'); + } + + public function testRejectsURIWithUnderscoreMethod() + { + $this->expectException(PageNotFoundException::class); + $this->expectExceptionMessage( + 'AutoRouterImproved prohibits access to the URI containing underscores ("dash_method")' + ); + + $router = $this->createNewAutoRouter(); + + $router->getRoute('dash-folder/dash-controller/dash_method', 'get'); + } + + public function testPermitsURIWithUnderscoreParam() + { + $router = $this->createNewAutoRouter(); + + [$directory, $controller, $method, $params] + = $router->getRoute('mycontroller/somemethod/a_b', 'get'); + + $this->assertNull($directory); + $this->assertSame('\\' . Mycontroller::class, $controller); + $this->assertSame('getSomemethod', $method); + $this->assertSame(['a_b'], $params); + } + + public function testDoesNotTranslateDashInParam() + { + $router = $this->createNewAutoRouter(); + + [$directory, $controller, $method, $params] + = $router->getRoute('mycontroller/somemethod/a-b', 'get'); + + $this->assertNull($directory); + $this->assertSame('\\' . Mycontroller::class, $controller); + $this->assertSame('getSomemethod', $method); + $this->assertSame(['a-b'], $params); + } } diff --git a/user_guide_src/source/changelogs/v4.4.0.rst b/user_guide_src/source/changelogs/v4.4.0.rst index dc1630e05a8f..85eb4517902e 100644 --- a/user_guide_src/source/changelogs/v4.4.0.rst +++ b/user_guide_src/source/changelogs/v4.4.0.rst @@ -151,6 +151,10 @@ Deprecations Bugs Fixed ********** +- **Auto Routing (Improved)**: In previous versions, when ``$translateURIDashes`` + is true, two URIs correspond to a single controller method, one URI for dashes + (e.g., **foo-bar**) and one URI for underscores (e.g., **foo_bar**). This bug + has been fixed. Now the URI for underscores (**foo_bar**) is not accessible. - **Output Buffering:** Bug fix with output buffering. See the repo's diff --git a/user_guide_src/source/incoming/routing.rst b/user_guide_src/source/incoming/routing.rst index 516bb23fe133..00de9cf790fd 100644 --- a/user_guide_src/source/incoming/routing.rst +++ b/user_guide_src/source/incoming/routing.rst @@ -607,6 +607,12 @@ URI segments when used in Auto Routing, thus saving you additional route entries .. literalinclude:: routing/049.php +.. note:: When using Auto Routing (Improved), prior to v4.4.0, if + ``$translateURIDashes`` is true, two URIs correspond to a single controller + method, one URI for dashes (e.g., **foo-bar**) and one URI for underscores + (e.g., **foo_bar**). This was incorrect behavior. Since v4.4.0, the URI for + underscores (**foo_bar**) is not accessible. + .. _use-defined-routes-only: Use Defined Routes Only diff --git a/user_guide_src/source/installation/upgrade_440.rst b/user_guide_src/source/installation/upgrade_440.rst index 5e7ab6856ce9..6a2c1960295d 100644 --- a/user_guide_src/source/installation/upgrade_440.rst +++ b/user_guide_src/source/installation/upgrade_440.rst @@ -60,6 +60,19 @@ by defining your own exception handler. See :ref:`custom-exception-handlers` for the detail. +Auto Routing (Improved) and translateURIDashes +============================================== + +When using Auto Routing (Improved) and ``$translateURIDashes`` is true +(``$routes->setTranslateURIDashes(true)``), in previous versions due to a bug +two URIs correspond to a single controller method, one URI for dashes +(e.g., **foo-bar**) and one URI for underscores (e.g., **foo_bar**). + +This bug was fixed and now URIs for underscores (**foo_bar**) is not accessible. + +If you have links to URIs for underscores (**foo_bar**), update them with URIs +for dashes (**foo-bar**). + Interface Changes =================