diff --git a/system/Router/Router.php b/system/Router/Router.php index f0f9555ad1ac..2073495a24b4 100644 --- a/system/Router/Router.php +++ b/system/Router/Router.php @@ -450,109 +450,125 @@ protected function checkRoutes(string $uri): bool $matchedKey = $routeKey; - // Are we dealing with a locale? - if (str_contains($routeKey, '{locale}')) { - $routeKey = str_replace('{locale}', '[^/]+', $routeKey); - } + // Route keys without PCRE metacharacters (including the pattern + // delimiter `#`) can be compared as plain strings, avoiding a + // PCRE call for static routes. + $isRegex = strpbrk($routeKey, '()[]?*+{}^$\\.|#') !== false; - // Does the RegEx match? - if (preg_match('#^' . $routeKey . '$#u', $uri, $matches)) { - // Is this route supposed to redirect to another? - if ($this->collection->isRedirect($routeKey)) { - // replacing matched route groups with references: post/([0-9]+) -> post/$1 - $redirectTo = preg_replace_callback('/(\([^\(]+\))/', static function (): string { - static $i = 1; - - return '$' . $i++; - }, is_array($handler) ? key($handler) : $handler); - - throw new RedirectException( - preg_replace('#\A' . $routeKey . '\z#u', $redirectTo, $uri), - $this->collection->getRedirectCode($routeKey), - ); + if (! $isRegex) { + if ($routeKey !== $uri) { + continue; } - // Store our locale so CodeIgniter object can - // assign it to the Request. - if (str_contains($matchedKey, '{locale}')) { - preg_match( - '#^' . str_replace('{locale}', '(?[^/]+)', $matchedKey) . '$#u', - $uri, - $matched, - ); - - if ($this->collection->shouldUseSupportedLocalesOnly() - && ! in_array($matched['locale'], config(App::class)->supportedLocales, true)) { - // Throw exception to prevent the autorouter, if enabled, - // from trying to find a route - throw PageNotFoundException::forLocaleNotSupported($matched['locale']); - } - $this->detectedLocale = $matched['locale']; - unset($matched); + $matches = [$uri]; + } else { + // Are we dealing with a locale? + if (str_contains($routeKey, '{locale}')) { + $routeKey = str_replace('{locale}', '[^/]+', $routeKey); } - // Are we using Closures? If so, then we need - // to collect the params into an array - // so it can be passed to the controller method later. - if (! is_string($handler) && is_callable($handler)) { - $this->controller = $handler; + // Does the RegEx match? + if (! preg_match('#^' . $routeKey . '$#u', $uri, $matches)) { + continue; + } + } - // Remove the original string from the matches array - array_shift($matches); + // Is this route supposed to redirect to another? + if ($this->collection->isRedirect($routeKey)) { + // replacing matched route groups with references: post/([0-9]+) -> post/$1 + $redirectTo = preg_replace_callback('/(\([^\(]+\))/', static function (): string { + static $i = 1; - $this->params = $matches; + return '$' . $i++; + }, is_array($handler) ? key($handler) : $handler); - $this->setMatchedRoute($matchedKey, $handler); + throw new RedirectException( + preg_replace('#\A' . $routeKey . '\z#u', $redirectTo, $uri), + $this->collection->getRedirectCode($routeKey), + ); + } - return true; - } + // Store our locale so CodeIgniter object can + // assign it to the Request. + if (str_contains($matchedKey, '{locale}')) { + preg_match( + '#^' . str_replace('{locale}', '(?[^/]+)', $matchedKey) . '$#u', + $uri, + $matched, + ); - if (str_contains($handler, '::')) { - [$controller, $methodAndParams] = explode('::', $handler); - } else { - $controller = $handler; - $methodAndParams = ''; + if ($this->collection->shouldUseSupportedLocalesOnly() + && ! in_array($matched['locale'], config(App::class)->supportedLocales, true)) { + // Throw exception to prevent the autorouter, if enabled, + // from trying to find a route + throw PageNotFoundException::forLocaleNotSupported($matched['locale']); } - // Checks `/` in controller name - if (str_contains($controller, '/')) { - throw RouterException::forInvalidControllerName($handler); + $this->detectedLocale = $matched['locale']; + unset($matched); + } + + // Are we using Closures? If so, then we need + // to collect the params into an array + // so it can be passed to the controller method later. + if (! is_string($handler) && is_callable($handler)) { + $this->controller = $handler; + + // Remove the original string from the matches array + array_shift($matches); + + $this->params = $matches; + + $this->setMatchedRoute($matchedKey, $handler); + + return true; + } + + if (str_contains($handler, '::')) { + [$controller, $methodAndParams] = explode('::', $handler); + } else { + $controller = $handler; + $methodAndParams = ''; + } + + // Checks `/` in controller name + if (str_contains($controller, '/')) { + throw RouterException::forInvalidControllerName($handler); + } + + if (str_contains($handler, '$') && str_contains($routeKey, '(')) { + // Checks dynamic controller + if (str_contains($controller, '$')) { + throw RouterException::forDynamicController($handler); } - if (str_contains($handler, '$') && str_contains($routeKey, '(')) { - // Checks dynamic controller - if (str_contains($controller, '$')) { - throw RouterException::forDynamicController($handler); + if (config(Routing::class)->multipleSegmentsOneParam === false) { + // Using back-references + $segments = explode('/', preg_replace('#\A' . $routeKey . '\z#u', $handler, $uri)); + } else { + if (str_contains($methodAndParams, '/')) { + [$method, $handlerParams] = explode('/', $methodAndParams, 2); + $params = explode('/', $handlerParams); + $handlerSegments = array_merge([$controller . '::' . $method], $params); + } else { + $handlerSegments = [$handler]; } - if (config(Routing::class)->multipleSegmentsOneParam === false) { - // Using back-references - $segments = explode('/', preg_replace('#\A' . $routeKey . '\z#u', $handler, $uri)); - } else { - if (str_contains($methodAndParams, '/')) { - [$method, $handlerParams] = explode('/', $methodAndParams, 2); - $params = explode('/', $handlerParams); - $handlerSegments = array_merge([$controller . '::' . $method], $params); - } else { - $handlerSegments = [$handler]; - } - - $segments = []; - - foreach ($handlerSegments as $segment) { - $segments[] = $this->replaceBackReferences($segment, $matches); - } + $segments = []; + + foreach ($handlerSegments as $segment) { + $segments[] = $this->replaceBackReferences($segment, $matches); } - } else { - $segments = explode('/', $handler); } + } else { + $segments = explode('/', $handler); + } - $this->setRequest($segments); + $this->setRequest($segments); - $this->setMatchedRoute($matchedKey, $handler); + $this->setMatchedRoute($matchedKey, $handler); - return true; - } + return true; } return false; diff --git a/tests/system/Router/RouterTest.php b/tests/system/Router/RouterTest.php index 6e22c95ed4f9..9f1ee2bbdb1e 100644 --- a/tests/system/Router/RouterTest.php +++ b/tests/system/Router/RouterTest.php @@ -1031,4 +1031,59 @@ public function testLogsRouteAttributeInstantiationFailureWithContext(): void $this->assertLogContains('error', 'Failed to instantiate route attribute "' . Filter::class . '" on "\Tests\Support\Router\Controllers\InvalidAttributeController::invalidMultipleFilters()":'); $this->assertLogContains('error', 'must be of type string'); } + + public function testStaticRoutesMatchControllerClosureAndNumericKey(): void + { + $collection = clone $this->collection; + $collection->resetRoutes(); + $collection->get('about-us', 'Home::about'); + $collection->get('contact_page', static fn (): string => 'contact-page-content'); + $collection->get('123', 'Home::numericRoute'); + + $router = new Router($collection, $this->request); + + $router->handle('about-us'); + $this->assertSame('\Home', $router->controllerName()); + $this->assertSame('about', $router->methodName()); + $this->assertSame([], $router->params()); + + $router->handle('contact_page'); + $controller = $router->controllerName(); + $this->assertInstanceOf(Closure::class, $controller); + $this->assertSame('contact-page-content', $controller()); + $this->assertSame([], $router->params()); + + $router->handle('123'); + $this->assertSame('\Home', $router->controllerName()); + $this->assertSame('numericRoute', $router->methodName()); + $this->assertSame([], $router->params()); + } + + public function testStaticRouteDoesNotMatchAdditionalSegments(): void + { + $collection = clone $this->collection; + $collection->resetRoutes(); + $collection->get('about-us', 'Home::about'); + + $router = new Router($collection, $this->request); + + $this->expectException(PageNotFoundException::class); + + $router->handle('about-us/team'); + } + + public function testEarlierDynamicRouteTakesPrecedenceOverLaterStaticRoute(): void + { + $collection = clone $this->collection; + $collection->resetRoutes(); + $collection->get('items/(:segment)', 'Items::detail/$1'); + $collection->get('items/new', 'Items::new'); + + $router = new Router($collection, $this->request); + $router->handle('items/new'); + + $this->assertSame('\Items', $router->controllerName()); + $this->assertSame('detail', $router->methodName()); + $this->assertSame(['new'], $router->params()); + } }