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: Inappropriate delimiter used in fillRouteParams #2353

Closed
korgoth opened this issue Oct 20, 2019 · 4 comments
Closed

Bug: Inappropriate delimiter used in fillRouteParams #2353

korgoth opened this issue Oct 20, 2019 · 4 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them waiting for info Issues or pull requests that need further clarification from the author
Milestone

Comments

@korgoth
Copy link

korgoth commented Oct 20, 2019

Describe the bug
Trying to use a simple regex in the routes config file such as (a|b|c) results in an ErrorException being thrown at SYSTEMPATH/Router/RouteCollection.php at line 1331 with the error of preg_match(): Unknown modifier
Upon investigation i see that the fillRouteParams function is trying to preg_match using pipe (|) as a delimiter, which is not appropriate as the pipe is often used in regular expressions.

CodeIgniter 4 version
rc.3

Affected module(s)
Router
Reverse routing

Expected behavior, and steps to reproduce if appropriate
I expected $route->get('(a|b|c)', "Controller::index/$1", ['as' => 'testname']);
To result in calling Controller:index with either a, b or c passed as first parameter.
The excption is thrown as soon as the route config files is being read.
P.S. I also tried escaping the pipe using \| but that resulted in CodeIgniter\Router\Exceptions\RouterException thrown in SYSTEMPATH/Router/Exceptions/RouterException.php at line 10

@korgoth korgoth added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 20, 2019
@lonnieezell lonnieezell added this to the 4.0.0 milestone Oct 22, 2019
@lonnieezell
Copy link
Member

I'm having problems recreating this one.

  1. Edited app/Config/Routes.php so the only route is: $routes->get('(a|b|c)', 'Home::index/$1', ['as' => 'testname']);
  2. Edited app/Controllers/Home.php so index looks like:
public function index($val = null)
{
	dd($val);
	return view('welcome_message');	
}
  1. Fired up $ php spark serve and navigated to localhost:8080/a. As expected, the output was a dump of a. Also navigated to /b,/c, /d with the first two showing the correct page and the 4th giving a 404 as expected.

What am I missing?

@lonnieezell
Copy link
Member

@korgoth can you provide any more insight?

@lonnieezell lonnieezell added the waiting for info Issues or pull requests that need further clarification from the author label Nov 26, 2019
@korgoth
Copy link
Author

korgoth commented Nov 26, 2019

Hello, @lonnieezell . Sorry for the delay but i am heavily on road last month.

After your first reply i made some more tests on my side and it seems that you are indeed correct. Having only this one route setup - things are working as expected. It turns out however, that my setup is a bit more complex and thats whats raising the exceptions. Here are some more details:

I wanted to have a (:any) route, setup as my last route, that captures everything thats not defined and sends it to a controller. Thats why i had to define a / controller to act as a home.
Here is my complete routes setup:

$routes->get('/', 'Home::index', ['as' => 'home']);
$routes->get('i/(:any)', 'Img::index');
$routes->get('g/(:any)', function($old_uri){ return redirect()->to('/'.url_title(urldecode($old_uri))); }); //REDIRECTS FROM OLD SITE
$routes->get('(a|b|c)', 'Galleries::index/$1', ['as' => 'galleries.list']);
$routes->get('chrono', 'Galleries::index', ['as' => 'galleries.list']);
$routes->get('seek', 'Galleries::search', ['as' => 'galleries.search']);
$routes->get('(:any)', 'Galleries::view/$1', ['as' => 'galleries.view']);

having it setup like this indeed produces correct results for (a|b|c) but throws an exception at /

ErrorException
Undefined offset: 0
SYSTEMPATH/Router/RouteCollection.php at line 1331

1324         // the appropriate places.
1325         foreach ($matches[0] as $index => $pattern)
1326         {
1327             // Ensure that the param we're inserting matches
1328             // the expected param type.
1329             $pos = strpos($from, $pattern);
1330 
1331             if (preg_match("|{$pattern}|", $params[$index]))
1332             {
1333                 $from = substr_replace($from, $params[$index], $pos, strlen($pattern));
1334             }
1335             else
1336             {
1337                 throw RouterException::forInvalidParameterType();
1338             }

Now it might be that ive misunderstood how routing works, if so - please tell me what am i getting wrong.
Other than that i hope this helps debug the situation as i think such a setup is frequently used

@lonnieezell
Copy link
Member

Sorry it took so long. Still can't recreate, though. Used these routes:

$routes->get('/', 'Home::index');
$routes->get('(a|b|c)', 'Home::printer/$1', ['as' => 'testname']);
$routes->get('(:any)', 'Home::catchall/$1');

And this controller:

public function index()
	{
		return view('welcome_message');
	}

	public function printer($val)
	{
		dd('printer', $val);
	}

	public function catchall($val)
	{
		dd('catchall', $val);
	}

Going to /, a, b, c, or d all take me to the expected method. I don't see any reason why it should when I look at your code. Sorry.

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 waiting for info Issues or pull requests that need further clarification from the author
Projects
None yet
Development

No branches or pull requests

2 participants