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.

Sign up for GitHub

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

Fix. router no set, Redirect can't work #3044

Merged
merged 1 commit into from
May 31, 2020
Merged

Fix. router no set, Redirect can't work #3044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions system/Router/RouteCollection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,18 +1439,22 @@ protected function create(string $verb, string $from, $to, array $options = null
$from = str_ireplace(':' . $tag, $pattern, $from);
}

// If no namespace found, add the default namespace
if (is_string($to) && (strpos($to, '\\') === false || strpos($to, '\\') > 0))
//If is redirect, No processing
if (! isset($options['redirect']))
{
$namespace = $options['namespace'] ?? $this->defaultNamespace;
$to = trim($namespace, '\\') . '\\' . $to;
}
// If no namespace found, add the default namespace
if (is_string($to) && (strpos($to, '\\') === false || strpos($to, '\\') > 0))
{
$namespace = $options['namespace'] ?? $this->defaultNamespace;
$to = trim($namespace, '\\') . '\\' . $to;
}

// Always ensure that we escape our namespace so we're not pointing to
// \CodeIgniter\Routes\Controller::method.
if (is_string($to))
{
$to = '\\' . ltrim($to, '\\');
// Always ensure that we escape our namespace so we're not pointing to
// \CodeIgniter\Routes\Controller::method.
if (is_string($to))
{
$to = '\\' . ltrim($to, '\\');
}
}

$name = $options['as'] ?? $from;
Expand Down
2 changes: 1 addition & 1 deletion system/Router/Router.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ protected function checkRoutes(string $uri): bool
// Is this route supposed to redirect to another?
if ($this->collection->isRedirect($key))
{
throw new RedirectException(key($val), $this->collection->getRedirectCode($key));
throw new RedirectException(is_array($val) ? key($val) : $val, $this->collection->getRedirectCode($key));
}
// Store our locale so CodeIgniter object can
// assign it to the Request.
Expand Down
114 changes: 97 additions & 17 deletions tests/system/CodeIgniterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,6 @@ public function tearDown(): void

//--------------------------------------------------------------------

public function testRunDefaultRoute()
{
$_SERVER['argv'] = [
'index.php',
'/',
];
$_SERVER['argc'] = 2;

ob_start();
$this->codeigniter->useSafeOutput(true)->run();
$output = ob_get_clean();

$this->assertStringContainsString('Welcome to CodeIgniter', $output);
}

//--------------------------------------------------------------------

public function testRunEmptyDefaultRoute()
{
$_SERVER['argv'] = [
Expand Down Expand Up @@ -300,4 +283,101 @@ public function testRunForceSecure()

$this->assertEquals('https://example.com', $response->getHeader('Location')->getValue());
}

public function testRunRedirectionWithNamed()
{
$_SERVER['argv'] = [
'index.php',
'example',
];
$_SERVER['argc'] = 2;
$_SERVER['REQUEST_URI'] = '/example';

// Inject mock router.
$routes = Services::routes();
$routes->add('pages/named', function () {
}, ['as' => 'name']);
$routes->addRedirect('example', 'name');

$router = Services::router($routes, Services::request());
Services::injectMock('router', $router);

ob_start();
$this->codeigniter->useSafeOutput(true)->run();
ob_get_clean();
$response = $this->getPrivateProperty($this->codeigniter, 'response');
$this->assertEquals('http://example.com/pages/named', $response->getHeader('Location')->getValue());
}

public function testRunRedirectionWithURI()
{
$_SERVER['argv'] = [
'index.php',
'example',
];
$_SERVER['argc'] = 2;
$_SERVER['REQUEST_URI'] = '/example';

// Inject mock router.
$routes = Services::routes();
$routes->add('pages/uri', function () {
});
$routes->addRedirect('example', 'pages/uri');

$router = Services::router($routes, Services::request());
Services::injectMock('router', $router);

ob_start();
$this->codeigniter->useSafeOutput(true)->run();
ob_get_clean();
$response = $this->getPrivateProperty($this->codeigniter, 'response');
$this->assertEquals('http://example.com/pages/uri', $response->getHeader('Location')->getValue());
}

/**
* @see https://github.com/codeigniter4/CodeIgniter4/issues/3041
*/
public function testRunRedirectionWithURINotSet()
{
$_SERVER['argv'] = [
'index.php',
'example',
];
$_SERVER['argc'] = 2;
$_SERVER['REQUEST_URI'] = '/example';

// Inject mock router.
$routes = Services::routes();
$routes->addRedirect('example', 'pages/notset');

$router = Services::router($routes, Services::request());
Services::injectMock('router', $router);

ob_start();
$this->codeigniter->useSafeOutput(true)->run();
ob_get_clean();
$response = $this->getPrivateProperty($this->codeigniter, 'response');
$this->assertEquals('http://example.com/pages/notset', $response->getHeader('Location')->getValue());
}

/**
* The method after all test, reset Servces:: config
* Can't use static::tearDownAfterClass. This will cause a buffer exception
* need improve
*/
public function testRunDefaultRoute()
{
$_SERVER['argv'] = [
'index.php',
'/',
];
$_SERVER['argc'] = 2;

ob_start();
$this->codeigniter->useSafeOutput(true)->run();
$output = ob_get_clean();

$this->assertStringContainsString('Welcome to CodeIgniter', $output);
}

}
5 changes: 3 additions & 2 deletions tests/system/Router/RouteCollectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -970,10 +970,11 @@ public function testAddRedirect()
{
$routes = $this->getCollector();

$routes->addRedirect('users', 'Users::index', 307);
//The second parameter is either the new URI to redirect to, or the name of a named route.
$routes->addRedirect('users', 'users/index', 307);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think instead of modify existing test, it needs new tests for new allowed usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This not a new allowed usage. The test is a mistak,It should not use Class::method.This is not what we accept


$expected = [
'users' => '\Users::index',
'users' => 'users/index',
];

$this->assertEquals($expected, $routes->getRoutes());
Expand Down