Merge pull request #7880 from kenjis/fix-reverse-route-nested-parentheses

fix: reverse routing causes ErrorException

Author: kenjis

system/Router/RouteCollection.php

@@ -1151,11 +1151,13 @@ public function environment(string $env, Closure $callback): RouteCollectionInte
     public function reverseRoute(string $search, ...$params)
     {
         // Named routes get higher priority.
-        foreach ($this->routesNames as $collection) {
+        foreach ($this->routesNames as $verb => $collection) {
             if (array_key_exists($search, $collection)) {
                 $routeKey = $collection[$search];
 
-                return $this->buildReverseRoute($routeKey, $params);
+                $from = $this->routes[$verb][$routeKey]['from'];
+
+                return $this->buildReverseRoute($from, $params);
             }
         }
 
@@ -1171,8 +1173,9 @@ public function reverseRoute(string $search, ...$params)
         // If it's not a named route, then loop over
         // all routes to find a match.
         foreach ($this->routes as $collection) {
-            foreach ($collection as $routeKey => $route) {
-                $to = $route['handler'];
+            foreach ($collection as $route) {
+                $to   = $route['handler'];
+                $from = $route['from'];
 
                 // ignore closures
                 if (! is_string($to)) {
@@ -1196,7 +1199,7 @@ public function reverseRoute(string $search, ...$params)
                     continue;
                 }
 
-                return $this->buildReverseRoute($routeKey, $params);
+                return $this->buildReverseRoute($from, $params);
             }
         }
 
@@ -1311,21 +1314,21 @@ protected function fillRouteParams(string $from, ?array $params = null): string
      * @param array $params One or more parameters to be passed to the route.
      *                      The last parameter allows you to set the locale.
      */
-    protected function buildReverseRoute(string $routeKey, array $params): string
+    protected function buildReverseRoute(string $from, array $params): string
     {
         $locale = null;
 
         // Find all of our back-references in the original route
-        preg_match_all('/\(([^)]+)\)/', $routeKey, $matches);
+        preg_match_all('/\(([^)]+)\)/', $from, $matches);
 
         if (empty($matches[0])) {
-            if (strpos($routeKey, '{locale}') !== false) {
+            if (strpos($from, '{locale}') !== false) {
                 $locale = $params[0] ?? null;
             }
 
-            $routeKey = $this->replaceLocale($routeKey, $locale);
+            $from = $this->replaceLocale($from, $locale);
 
-            return '/' . ltrim($routeKey, '/');
+            return '/' . ltrim($from, '/');
         }
 
         // Locale is passed?
@@ -1336,25 +1339,31 @@ protected function buildReverseRoute(string $routeKey, array $params): string
 
         // Build our resulting string, inserting the $params in
         // the appropriate places.
-        foreach ($matches[0] as $index => $pattern) {
+        foreach ($matches[0] as $index => $placeholder) {
             if (! isset($params[$index])) {
                 throw new InvalidArgumentException(
-                    'Missing argument for "' . $pattern . '" in route "' . $routeKey . '".'
+                    'Missing argument for "' . $placeholder . '" in route "' . $from . '".'
                 );
             }
+
+            // Remove `(:` and `)` when $placeholder is a placeholder.
+            $placeholderName = substr($placeholder, 2, -1);
+            // or maybe $placeholder is not a placeholder, but a regex.
+            $pattern = $this->placeholders[$placeholderName] ?? $placeholder;
+
             if (! preg_match('#^' . $pattern . '$#u', $params[$index])) {
                 throw RouterException::forInvalidParameterType();
             }
 
             // Ensure that the param we're inserting matches
             // the expected param type.
-            $pos      = strpos($routeKey, $pattern);
-            $routeKey = substr_replace($routeKey, $params[$index], $pos, strlen($pattern));
+            $pos  = strpos($from, $placeholder);
+            $from = substr_replace($from, $params[$index], $pos, strlen($placeholder));
         }
 
-        $routeKey = $this->replaceLocale($routeKey, $locale);
+        $from = $this->replaceLocale($from, $locale);
 
-        return '/' . ltrim($routeKey, '/');
+        return '/' . ltrim($from, '/');
     }
 
     /**

tests/system/Helpers/URLHelper/MiscUrlTest.php

@@ -906,13 +906,50 @@ public function testUrlToWithSupportedLocaleInRoute(): void
         );
     }
 
+    public function testUrlToWithNamedRouteWithNestedParentheses(): void
+    {
+        Services::createRequest(new App());
+        $routes = service('routes');
+
+        // The route will be:
+        //   docs/(master|\d+\.(?:\d+|x))/([a-z0-9-]+)
+        $routes->addPlaceholder([
+            'version' => 'master|\d+\.(?:\d+|x)',
+            'page'    => '[a-z0-9-]+',
+        ]);
+        $routes->get('docs/(:version)/(:page)', static function () {
+            echo 'Test the documentation segment';
+        }, ['as' => 'docs.version']);
+
+        $this->assertSame(
+            'http://example.com/index.php/docs/10.9/install',
+            url_to('docs.version', '10.9', 'install')
+        );
+    }
+
+    public function testUrlToWithRouteWithNestedParentheses(): void
+    {
+        Services::createRequest(new App());
+        $routes = service('routes');
+
+        // The route will be:
+        //   images/(^.*\.(?:jpg|png)$)
+        $routes->addPlaceholder('imgFileExt', '^.*\.(?:jpg|png)$');
+        $routes->get('images/(:imgFileExt)', 'Images::getFile/$1');
+
+        $this->assertSame(
+            'http://example.com/index.php/images/test.jpg',
+            url_to('Images::getFile', 'test.jpg')
+        );
+    }
+
     /**
      * @see https://github.com/codeigniter4/CodeIgniter4/issues/7651
      */
     public function testUrlToMissingArgument(): void
     {
         $this->expectException(InvalidArgumentException::class);
-        $this->expectExceptionMessage('Missing argument for "([a-zA-Z]+)" in route "([a-zA-Z]+)/login".');
+        $this->expectExceptionMessage('Missing argument for "(:alpha)" in route "(:alpha)/login".');
 
         $routes = Services::routes();
         $routes->group('(:alpha)', static function ($routes): void {