Skip to content

Commit 3fd3863

Browse files
sneakyimpsneakyimp
authored
Updated Router to properly translate URI dashes that map to controller subdirs
* updated Router to properly translate uri dashes that map to controller subdirectories. fix issue #4294 * changed Router::validateRequest to Router::scanControllers, added isValidSegment, tweaks to enforce PSR4 compliance in directory/namespace segments * PHPCBF cleanup * added numerous dash tests, etc. to RouterTest * Router fixes: fix errant return true in setDirectory, fix (bool) cast in isValidSegment, add back deprecated validateRequest function, add RouterTest methods for better code coverage * fixed broken assertions in RouterTest::testRouterPriorDirectory * added space after boolean cast as requested by paulbalandan
1 parent 93f5c11 commit 3fd3863

File tree

2 files changed

+256
-19
lines changed

2 files changed

+256
-19
lines changed

‎system/Router/Router.php‎

Lines changed: 75 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -498,7 +498,7 @@ public function autoRoute(string $uri)
498498
{
499499
$segments = explode('/', $uri);
500500

501-
$segments = $this->validateRequest($segments);
501+
$segments = $this->scanControllers($segments);
502502

503503
// If we don't have any segments left - try the default controller;
504504
// WARNING: Directories get shifted out of the segments array.
@@ -512,6 +512,12 @@ public function autoRoute(string $uri)
512512
$this->controller = ucfirst(array_shift($segments));
513513
}
514514

515+
$controllerName = $this->controllerName();
516+
if (! $this->isValidSegment($controllerName))
517+
{
518+
throw new PageNotFoundException($this->controller . ' is not a valid controller name');
519+
}
520+
515521
// Use the method name if it exists.
516522
// If it doesn't, no biggie - the default method name
517523
// has already been set.
@@ -526,7 +532,6 @@ public function autoRoute(string $uri)
526532
}
527533

528534
$defaultNamespace = $this->collection->getDefaultNamespace();
529-
$controllerName = $this->controllerName();
530535
if ($this->collection->getHTTPVerb() !== 'cli')
531536
{
532537
$controller = '\\' . $defaultNamespace;
@@ -573,32 +578,61 @@ public function autoRoute(string $uri)
573578
//--------------------------------------------------------------------
574579

575580
/**
576-
* Attempts to validate the URI request and determine the controller path.
581+
* Scans the controller directory, attempting to locate a controller matching the supplied uri $segments
577582
*
578583
* @param array $segments URI segments
579584
*
580-
* @return array URI segments
585+
* @return array returns an array of remaining uri segments that don't map onto a directory
586+
*
587+
* @deprecated this function name does not properly describe its behavior so it has been deprecated
581588
*/
582589
protected function validateRequest(array $segments): array
590+
{
591+
return $this->scanControllers($segments);
592+
}
593+
594+
//--------------------------------------------------------------------
595+
596+
/**
597+
* Scans the controller directory, attempting to locate a controller matching the supplied uri $segments
598+
*
599+
* @param array $segments URI segments
600+
*
601+
* @return array returns an array of remaining uri segments that don't map onto a directory
602+
*/
603+
protected function scanControllers(array $segments): array
583604
{
584605
$segments = array_filter($segments, function ($segment) {
585-
// @phpstan-ignore-next-line
586-
return ! empty($segment) || ($segment !== '0' || $segment !== 0);
606+
return $segment !== '';
587607
});
608+
// numerically reindex the array, removing gaps
588609
$segments = array_values($segments);
589610

590-
$c = count($segments);
591-
$directoryOverride = isset($this->directory);
611+
// if a prior directory value has been set, just return segments and get out of here
612+
if (isset($this->directory))
613+
{
614+
return $segments;
615+
}
592616

593617
// Loop through our segments and return as soon as a controller
594618
// is found or when such a directory doesn't exist
619+
$c = count($segments);
595620
while ($c-- > 0)
596621
{
597-
$test = $this->directory . ucfirst($this->translateURIDashes === true ? str_replace('-', '_', $segments[0]) : $segments[0]);
622+
$segmentConvert = ucfirst($this->translateURIDashes === true ? str_replace('-', '_', $segments[0]) : $segments[0]);
623+
// as soon as we encounter any segment that is not PSR-4 compliant, stop searching
624+
if (! $this->isValidSegment($segmentConvert))
625+
{
626+
return $segments;
627+
}
628+
629+
$test = APPPATH . 'Controllers/' . $this->directory . $segmentConvert;
598630

599-
if (! is_file(APPPATH . 'Controllers/' . $test . '.php') && $directoryOverride === false && is_dir(APPPATH . 'Controllers/' . $this->directory . ucfirst($segments[0])))
631+
// as long as each segment is *not* a controller file but does match a directory, add it to $this->directory
632+
if (! is_file($test . '.php') && is_dir($test))
600633
{
601-
$this->setDirectory(array_shift($segments), true);
634+
$this->setDirectory($segmentConvert, true, false);
635+
array_shift($segments);
602636
continue;
603637
}
604638

@@ -614,29 +648,53 @@ protected function validateRequest(array $segments): array
614648
/**
615649
* Sets the sub-directory that the controller is in.
616650
*
617-
* @param string|null $dir
618-
* @param boolean|false $append
651+
* @param string|null $dir
652+
* @param boolean $append
653+
* @param boolean $validate if true, checks to make sure $dir consists of only PSR4 compliant segments
619654
*/
620-
public function setDirectory(string $dir = null, bool $append = false)
655+
public function setDirectory(string $dir = null, bool $append = false, bool $validate = true)
621656
{
622657
if (empty($dir))
623658
{
624659
$this->directory = null;
625660
return;
626661
}
627662

628-
$dir = ucfirst($dir);
663+
if ($validate)
664+
{
665+
$segments = explode('/', trim($dir, '/'));
666+
foreach ($segments as $segment)
667+
{
668+
if (! $this->isValidSegment($segment))
669+
{
670+
return;
671+
}
672+
}
673+
}
629674

630675
if ($append !== true || empty($this->directory))
631676
{
632-
$this->directory = str_replace('.', '', trim($dir, '/')) . '/';
677+
$this->directory = trim($dir, '/') . '/';
633678
}
634679
else
635680
{
636-
$this->directory .= str_replace('.', '', trim($dir, '/')) . '/';
681+
$this->directory .= trim($dir, '/') . '/';
637682
}
638683
}
639684

685+
/**
686+
* Returns true if the supplied $segment string represents a valid PSR-4 compliant namespace/directory segment
687+
*
688+
* regex comes from https://www.php.net/manual/en/language.variables.basics.php
689+
*
690+
* @param string $segment
691+
* @return boolean
692+
*/
693+
private function isValidSegment(string $segment): bool
694+
{
695+
return (bool) preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/', $segment);
696+
}
697+
640698
//--------------------------------------------------------------------
641699

642700
/**

‎tests/system/Router/RouterTest.php‎

Lines changed: 181 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use CodeIgniter\HTTP\IncomingRequest;
66
use CodeIgniter\Test\CIUnitTestCase;
77
use Config\Modules;
8+
use CodeIgniter\Exceptions\PageNotFoundException;
89

910
class RouterTest extends CIUnitTestCase
1011
{
@@ -81,9 +82,9 @@ public function testZeroAsURIPath()
8182
{
8283
$router = new Router($this->collection, $this->request);
8384

84-
$router->handle('0');
85+
$this->expectException(PageNotFoundException::class);
8586

86-
$this->assertEquals('0', $router->controllerName());
87+
$router->handle('0');
8788
}
8889

8990
//--------------------------------------------------------------------
@@ -231,6 +232,153 @@ public function testAutoRouteFindsControllerWithSubfolder()
231232

232233
//--------------------------------------------------------------------
233234

235+
public function testAutoRouteFindsDashedSubfolder()
236+
{
237+
$router = new Router($this->collection, $this->request);
238+
$router->setTranslateURIDashes(true);
239+
240+
mkdir(APPPATH . 'Controllers/Dash_folder');
241+
242+
$router->autoRoute('dash-folder/mycontroller/somemethod');
243+
244+
rmdir(APPPATH . 'Controllers/Dash_folder');
245+
246+
$this->assertEquals('Dash_folder/', $router->directory());
247+
$this->assertEquals('Mycontroller', $router->controllerName());
248+
$this->assertEquals('somemethod', $router->methodName());
249+
}
250+
251+
//--------------------------------------------------------------------
252+
253+
public function testAutoRouteFindsDashedController()
254+
{
255+
$router = new Router($this->collection, $this->request);
256+
$router->setTranslateURIDashes(true);
257+
258+
mkdir(APPPATH . 'Controllers/Dash_folder');
259+
file_put_contents(APPPATH . 'Controllers/Dash_folder/Dash_controller.php', '');
260+
261+
$router->autoRoute('dash-folder/dash-controller/somemethod');
262+
263+
unlink(APPPATH . 'Controllers/Dash_folder/Dash_controller.php');
264+
rmdir(APPPATH . 'Controllers/Dash_folder');
265+
266+
$this->assertEquals('Dash_folder/', $router->directory());
267+
$this->assertEquals('Dash_controller', $router->controllerName());
268+
$this->assertEquals('somemethod', $router->methodName());
269+
}
270+
271+
//--------------------------------------------------------------------
272+
273+
public function testAutoRouteFindsDashedMethod()
274+
{
275+
$router = new Router($this->collection, $this->request);
276+
$router->setTranslateURIDashes(true);
277+
278+
mkdir(APPPATH . 'Controllers/Dash_folder');
279+
file_put_contents(APPPATH . 'Controllers/Dash_folder/Dash_controller.php', '');
280+
281+
$router->autoRoute('dash-folder/dash-controller/dash-method');
282+
283+
unlink(APPPATH . 'Controllers/Dash_folder/Dash_controller.php');
284+
rmdir(APPPATH . 'Controllers/Dash_folder');
285+
286+
$this->assertEquals('Dash_folder/', $router->directory());
287+
$this->assertEquals('Dash_controller', $router->controllerName());
288+
$this->assertEquals('dash_method', $router->methodName());
289+
}
290+
291+
//--------------------------------------------------------------------
292+
293+
public function testAutoRouteFindsDefaultDashFolder()
294+
{
295+
$router = new Router($this->collection, $this->request);
296+
$router->setTranslateURIDashes(true);
297+
298+
mkdir(APPPATH . 'Controllers/Dash_folder');
299+
300+
$router->autoRoute('dash-folder');
301+
302+
rmdir(APPPATH . 'Controllers/Dash_folder');
303+
304+
$this->assertEquals('Dash_folder/', $router->directory());
305+
$this->assertEquals('Home', $router->controllerName());
306+
$this->assertEquals('index', $router->methodName());
307+
}
308+
309+
//--------------------------------------------------------------------
310+
311+
public function testAutoRouteFindsMByteDir()
312+
{
313+
$router = new Router($this->collection, $this->request);
314+
$router->setTranslateURIDashes(true);
315+
316+
mkdir(APPPATH . 'Controllers/Φ');
317+
318+
$router->autoRoute('Φ');
319+
320+
rmdir(APPPATH . 'Controllers/Φ');
321+
322+
$this->assertEquals('Φ/', $router->directory());
323+
$this->assertEquals('Home', $router->controllerName());
324+
$this->assertEquals('index', $router->methodName());
325+
}
326+
327+
//--------------------------------------------------------------------
328+
329+
public function testAutoRouteFindsMByteController()
330+
{
331+
$router = new Router($this->collection, $this->request);
332+
$router->setTranslateURIDashes(true);
333+
334+
file_put_contents(APPPATH . 'Controllers/Φ', '');
335+
336+
$router->autoRoute('Φ');
337+
338+
unlink(APPPATH . 'Controllers/Φ');
339+
340+
$this->assertEquals('Φ', $router->controllerName());
341+
$this->assertEquals('index', $router->methodName());
342+
}
343+
344+
//--------------------------------------------------------------------
345+
346+
public function testAutoRouteRejectsSingleDot()
347+
{
348+
$router = new Router($this->collection, $this->request);
349+
$router->setTranslateURIDashes(true);
350+
351+
$this->expectException(PageNotFoundException::class);
352+
353+
$router->autoRoute('.');
354+
}
355+
356+
//--------------------------------------------------------------------
357+
358+
public function testAutoRouteRejectsDoubleDot()
359+
{
360+
$router = new Router($this->collection, $this->request);
361+
$router->setTranslateURIDashes(true);
362+
363+
$this->expectException(PageNotFoundException::class);
364+
365+
$router->autoRoute('..');
366+
}
367+
368+
//--------------------------------------------------------------------
369+
370+
public function testAutoRouteRejectsMidDot()
371+
{
372+
$router = new Router($this->collection, $this->request);
373+
$router->setTranslateURIDashes(true);
374+
375+
$this->expectException(PageNotFoundException::class);
376+
377+
$router->autoRoute('Foo.bar');
378+
}
379+
380+
//--------------------------------------------------------------------
381+
234382
public function testDetectsLocales()
235383
{
236384
$router = new Router($this->collection, $this->request);
@@ -575,4 +723,35 @@ public function testRegularExpressionPlaceholderWithUnicode()
575723
];
576724
$this->assertEquals($expected, $router->params());
577725
}
726+
727+
public function testRouterPriorDirectory()
728+
{
729+
$router = new Router($this->collection, $this->request);
730+
731+
$router->setDirectory('foo/bar/baz', false, true);
732+
$router->handle('Some_controller/some_method/param1/param2/param3');
733+
734+
$this->assertEquals('foo/bar/baz/', $router->directory());
735+
$this->assertEquals('Some_controller', $router->controllerName());
736+
$this->assertEquals('some_method', $router->methodName());
737+
}
738+
739+
public function testSetDirectoryValid()
740+
{
741+
$router = new Router($this->collection, $this->request);
742+
$router->setDirectory('foo/bar/baz', false, true);
743+
744+
$this->assertEquals('foo/bar/baz/', $router->directory());
745+
}
746+
747+
public function testSetDirectoryInvalid()
748+
{
749+
$router = new Router($this->collection, $this->request);
750+
$router->setDirectory('foo/bad-segment/bar', false, true);
751+
752+
$internal = $this->getPrivateProperty($router, 'directory');
753+
754+
$this->assertNull($internal);
755+
$this->assertEquals('', $router->directory());
756+
}
578757
}

0 commit comments

Comments
 (0)