codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.25k stars 1.88k forks source link

Bug: Routes with {locale} before the actual path are not loaded properly #7609

Closed SvetoslavStefanov closed 1 year ago

SvetoslavStefanov commented 1 year ago

PHP Version

8.0

CodeIgniter4 Version

4.3

CodeIgniter4 Installation Method

Composer (using codeigniter4/appstarter)

Which operating systems have you tested for this bug?

Windows

Which server did you use?

apache

Database

MySQL

What happened?

I'm having a few sets of routes and localization on my website. The problem comes when I put {locale} before the actual route, because it reads everything there as a locale. Let me give an example:

Steps to Reproduce

$routes->useSupportedLocalesOnly(true);

$routes->get('userEmployee/edit/(:num)', [HomeController::class, 'index']); //route 1
$routes->get('{locale}/userEmployee/edit/(:num)', [HomeController::class, 'index']); //route 2

$routes->get('admin/userEmployee/edit/(:num)', [AdminHomeController::class, 'index']); //route 3
$routes->get('admin/{locale}/userEmployee/edit/(:num)', [AdminHomeController::class, 'index']); //route4

Expected Output

In this case route 3 wouldn't be accessible, because I'll get an error "language not supported" - it will try to match "admin" as a language in this case. I'm expecting to be able to access all my routes, if they're not duplicated.

In this case admin is wrongly matched as a {locale}, which is causing all the issues here.

Anything else?

No response

kenjis commented 1 year ago

This is not a bug. Your route definitions are wrong.

Routes are registered in the routing table in the order in which they are defined. This means that when a URI is accessed, the first matching route will be executed. https://www.codeigniter.com/user_guide/incoming/routing.html#route-priority

Your routes are the following:

+--------+-------------------------------------------+------+------------------------------------------------+----------------+---------------+
| Method | Route                                     | Name | Handler                                        | Before Filters | After Filters |
+--------+-------------------------------------------+------+------------------------------------------------+----------------+---------------+
| GET    | /                                         | »    | \App\Controllers\Home::index                   |                | toolbar       |
| GET    | userEmployee/edit/([0-9]+)                | »    | \App\Controllers\HomeController::index/$1      |                | toolbar       |
| GET    | {locale}/userEmployee/edit/([0-9]+)       | »    | \App\Controllers\HomeController::index/$1      | <unknown>      | <unknown>     |
| GET    | admin/userEmployee/edit/([0-9]+)          | »    | \App\Controllers\AdminHomeController::index/$1 | <unknown>      | <unknown>     |
| GET    | admin/{locale}/userEmployee/edit/([0-9]+) | »    | \App\Controllers\AdminHomeController::index/$1 | <unknown>      | <unknown>     |
+--------+-------------------------------------------+------+------------------------------------------------+----------------+---------------+

{locale} means [^/]+ in regex.

If you navigate to admin/userEmployee/edit/1, the route {locale}/userEmployee/edit/([0-9]+) (= [^/]+/userEmployee/edit/([0-9]+)) will match.

kenjis commented 1 year ago

We use GitHub issues to track BUGS and to track approved DEVELOPMENT work packages. We use our forum to provide SUPPORT and to discuss FEATURE REQUESTS.

SvetoslavStefanov commented 1 year ago

Why this is not a bug, since I have clearly defined routes? I don't think that having {locale}/some-route-here is acceptable to mean that anything/some-route-here is an invalid route.

If I'm allowed to define this in Config/Routes.php, this means that I'm expected to be able to access that route, right? Or at least to be presented with an error that this is not allowed route.

kenjis commented 1 year ago

If I'm allowed to define this in Config/Routes.php, this means that I'm expected to be able to access that route, right?

No. Routes are registered in the routing table in the order in which they are defined. If there are more than one matching route for a URI, the first matching route will be executed.

Or at least to be presented with an error that this is not allowed route.

It is not implemented that way. If you can implement that way, feel free to send a PR. https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md

SvetoslavStefanov commented 1 year ago

They're clearly different routes and they shouldn't be treated as the same in the first place. They're treated as the same, which is the bug I'm reporting here.

The way {locale} is being matched is wrong. It should take into account that there are other routes, before proceeding forward in the "matching" process.

kenjis commented 1 year ago

They are different routes. But the {locale} route matches first, because you define it first. {locale} is a wild card.