From 472ee9f46f8a9fc0033b0c90ecafbc19113b1c93 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Sun, 1 Nov 2026 22:42:08 +0100 Subject: [PATCH] perf: add fast-path for static routes in Router::checkRoutes() Avoid executing PCRE for static routes by checking for PCRE metacharacters (including pattern delimiter '#') using strpbrk(). If none are present, perform a direct string comparison against the request URI. This reduces matching overhead from O(N) PCRE invocations to O(1) string checks for static routes while strictly preserving registration precedence and existing handler, Closure, redirect, and locale behavior. --- system/Router/Router.php | 180 ++++++++++++++++------------- tests/system/Router/RouterTest.php | 55 +++++++++ 2 files changed, 153 insertions(+), 82 deletions(-) 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()); + } }