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

Routing to hardcoded ids does not work #1838

Closed
bintzandt opened this Issue Mar 14, 2019 · 1 comment

Comments

Projects
None yet
1 participant
@bintzandt
Copy link
Contributor

bintzandt commented Mar 14, 2019

Describe the bug
The router should allow me to add routes in the following way:
$routes->get( 'abcd/efgh', 'Pages::id/123' )
However, in the default Router (Codeigniter\Router), all the / in the$to parameter get overwritten by \. This breaks the functionality of the route since it starts looking at the Pages controller for the function id\123 (which does not exist).

Your own guidelines even have an example that has this kind of route in it.
$routes->add('blog/joe', 'Blogs::users/34');
I have not actually tested this, but I guess that it fails for the same reasons as why my route is failing.

As far as I can tell, the way I want to route corresponds roughly with the routing described in the tutorial.

This problem seems to occur to the following lines of code in the checkRoutes( string $url ) function (in the Router, lines 471 to 474).

elseif (strpos($val, '/') !== false)
	{
		$val = str_replace('/', '\\', $val);
	}

These lines translate all the / in $val to \. I don't know why this is done, but commenting those four lines fixes the problem for me (although I don't know what other consequences there are...).

CodeIgniter 4 version
Which version (and branch, if applicable) the bug is in.
The first Codeigniter beta version.

Expected behavior, and steps to reproduce if appropriate
I expect that my routes are routed to the controller with the 123 passed as the $id parameter.

Context

  • OS: [MacOS]
  • Using php spark serve
  • PHP version [7.3]
@bintzandt

This comment has been minimized.

Copy link
Contributor Author

bintzandt commented Mar 14, 2019

I took the liberty of creating a PR that removes the translation of / into \ in the $to part of the routes. However, I cannot oversee all the consequences of this PR so please only merge if it does not break anything else (although I cannot quickly see what it would break 👍 ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.