Skip to content

Commit

Permalink
Merge pull request #1355 from jim-parry/fix/routes
Browse files Browse the repository at this point in the history
Handle duplicate HTTP verb and generic rules properly
  • Loading branch information
jim-parry authored Oct 25, 2018
2 parents 6870339 + efc7338 commit 9a12631
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 24 deletions.
54 changes: 30 additions & 24 deletions system/Router/RouteCollection.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php namespace CodeIgniter\Router;
<?php
namespace CodeIgniter\Router;

/**
* CodeIgniter
Expand Down Expand Up @@ -132,16 +133,16 @@ class RouteCollection implements RouteCollectionInterface
* @var array
*/
protected $routes = [
'*' => [],
'options' => [],
'get' => [],
'head' => [],
'post' => [],
'put' => [],
'delete' => [],
'trace' => [],
'connect' => [],
'cli' => [],
'*' => [],
'options' => [],
'get' => [],
'head' => [],
'post' => [],
'put' => [],
'delete' => [],
'trace' => [],
'connect' => [],
'cli' => [],
];

/**
Expand Down Expand Up @@ -383,7 +384,8 @@ public function get404Override()
*/
protected function discoverRoutes()
{
if ($this->didDiscover) return;
if ($this->didDiscover)
return;

// We need this var in local scope
// so route files can access it.
Expand Down Expand Up @@ -520,8 +522,11 @@ public function getRoutes($verb = null): array
{
// Keep current verb's routes at the beginning so they're matched
// before any of the generic, "add" routes.
$collection = array_merge($this->routes[$verb], $this->routes['*']);

if (isset($this->routes['*']))
{
$extraRules = array_diff_key($this->routes['*'], $this->routes[$verb]);
$collection = array_merge($this->routes[$verb], $extraRules);
}
foreach ($collection as $r)
{
$key = key($r['route']);
Expand Down Expand Up @@ -806,13 +811,13 @@ public function resource(string $name, array $options = null): RouteCollectionIn

$methods = isset($options['only']) ? is_string($options['only']) ? explode(',', $options['only']) : $options['only'] : ['index', 'show', 'create', 'update', 'delete', 'new', 'edit'];

if(isset($options['except']))
if (isset($options['except']))
{
$options['except'] = is_array($options['except']) ? $options['except'] : explode(',', $options['except']);
$c = count($methods);
for($i = 0; $i < $c; $i++)
for ($i = 0; $i < $c; $i ++)
{
if(in_array($methods[$i], $options['except']))
if (in_array($methods[$i], $options['except']))
{
unset($methods[$i]);
}
Expand All @@ -822,16 +827,16 @@ public function resource(string $name, array $options = null): RouteCollectionIn
if (in_array('index', $methods))
$this->get($name, $new_name . '::index', $options);
if (in_array('new', $methods))
$this->get($name. '/new', $new_name . '::new', $options);
$this->get($name . '/new', $new_name . '::new', $options);
if (in_array('edit', $methods))
$this->get($name . '/' . $id. '/edit', $new_name . '::edit/$1', $options);
$this->get($name . '/' . $id . '/edit', $new_name . '::edit/$1', $options);
if (in_array('show', $methods))
$this->get($name . '/' . $id, $new_name . '::show/$1', $options);
if (in_array('create', $methods))
$this->post($name, $new_name . '::create', $options);
if (in_array('update', $methods))
$this->put($name . '/' . $id, $new_name . '::update/$1', $options);
$this->patch($name . '/' . $id, $new_name . '::update/$1', $options);
$this->patch($name . '/' . $id, $new_name . '::update/$1', $options);
if (in_array('delete', $methods))
$this->delete($name . '/' . $id, $new_name . '::delete/$1', $options);

Expand Down Expand Up @@ -1137,7 +1142,7 @@ public function isFiltered(string $search): bool
*/
public function getFilterForRoute(string $search): string
{
if (! $this->isFiltered($search))
if ( ! $this->isFiltered($search))
{
return '';
}
Expand Down Expand Up @@ -1211,10 +1216,10 @@ protected function create(string $verb, string $from, $to, array $options = null
$from = trim($from, '/');
}

$options = array_merge((array)$this->currentOptions, (array)$options);
$options = array_merge((array) $this->currentOptions, (array) $options);

// Hostname limiting?
if (! empty($options['hostname']))
if ( ! empty($options['hostname']))
{
// @todo determine if there's a way to whitelist hosts?
if (strtolower($_SERVER['HTTP_HOST']) != strtolower($options['hostname']))
Expand Down Expand Up @@ -1245,7 +1250,8 @@ protected function create(string $verb, string $from, $to, array $options = null
for ($i = (int) $options['offset'] + 1; $i < (int) $options['offset'] + 7; $i ++ )
{
$to = preg_replace_callback(
'/\$X/', function ($m) use ($i) {
'/\$X/', function ($m) use ($i)
{
return '$' . $i;
}, $to, 1
);
Expand Down
19 changes: 19 additions & 0 deletions tests/system/Router/RouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -301,4 +301,23 @@ public function testMatchesCorrectlyWithMixedVerbs()
$this->assertEquals('\Pages', $router->controllerName());
$this->assertEquals('view', $router->methodName());
}
//--------------------------------------------------------------------

/**
* @see https://github.com/bcit-ci/CodeIgniter4/issues/1354
*/
public function testRouteOrder()
{
$this->collection->setHTTPVerb('post');

$this->collection->post('auth', 'Main::auth_post');
$this->collection->add('auth', 'Main::index');

$router = new Router($this->collection);

$router->handle('auth');
$this->assertEquals('\Main', $router->controllerName());
$this->assertEquals('auth_post', $router->methodName());

}
}

0 comments on commit 9a12631

Please sign in to comment.