Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Reverse routing fails for routes with regex containing parenthesis #7237

Closed
csg-luke opened this issue Feb 9, 2023 · 4 comments · Fixed by #7880
Closed

Bug: Reverse routing fails for routes with regex containing parenthesis #7237

csg-luke opened this issue Feb 9, 2023 · 4 comments · Fixed by #7880
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@csg-luke
Copy link

csg-luke commented Feb 9, 2023

PHP Version

8.0

CodeIgniter4 Version

4.3.1

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

MariaDB 10.6

What happened?

Reverse routing is failing for routes with regex containing parenthesis

Error: preg_match(): Compilation failed: missing closing parenthesis at offset 19,
vendor/codeigniter4/framework/system/Router/RouteCollection.php
line: 1238

Looking at the RouteCollection.php line 1211
preg_match_all('/\(([^)]+)\)/', $from, $matches);
It appears the regex is only matching to the first parenthesis and losing the rest.

Steps to Reproduce

$routes->addPlaceholder('imgFileExt', '^.*\.(?:jpg|png)$');
$routes->get('images/(:imgFileExt)', 'Images::getFile/$1');
$file = 'test.jpg';
url_to('Images::getFile', $file)

Expected Output

<base_url>/images/test.jpg

Anything else?

No response

@csg-luke csg-luke added the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 9, 2023
@totoprayogo1916
Copy link
Contributor

working with:

$routes->addPlaceholder('imgFileExt', '.*(?:jpg|png)');

@kenjis
Copy link
Member

kenjis commented Feb 9, 2023

I don't want to permit Route like images/(^.*\.(?:jpg|png)$).
I think it would be fine to say that regular expressions with double parentheses such as this are not supported.

+--------+----------------------------+------+-------------------------------------+----------------+---------------+
| Method | Route                      | Name | Handler                             | Before Filters | After Filters |
+--------+----------------------------+------+-------------------------------------+----------------+---------------+
| GET    | /                          | »    | \App\Controllers\Home::index        |                | toolbar       |
| GET    | images/(^.*\.(?:jpg|png)$) | »    | \App\Controllers\Images::getFile/$1 | <unknown>      | <unknown>     |
+--------+----------------------------+------+-------------------------------------+----------------+---------------+

@kenjis kenjis removed the bug Verified issues on the current code behavior or pull requests that will fix them label Feb 19, 2023
@kenjis
Copy link
Member

kenjis commented Mar 25, 2023

Workaround:

$routes->addPlaceholder('img', '.*\.jpg|.*\.png');
$routes->get('images/(:img)', 'Home::index/$1');

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Mar 25, 2023
@kenjis
Copy link
Member

kenjis commented Aug 30, 2023

Try #7880

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants