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

Fixed multiple trailing slashes issues. #618

Merged
merged 1 commit into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion flight/Engine.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public function init(): void
}

// Set case-sensitivity
$self->router()->case_sensitive = $self->get('flight.case_sensitive');
$self->router()->caseSensitive = $self->get('flight.case_sensitive');
// Set Content-Length
$self->response()->content_length = $self->get('flight.content_length');
// This is to maintain legacy handling of output buffering
Expand Down
13 changes: 10 additions & 3 deletions flight/net/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,24 @@ public function __construct(string $pattern, $callback, array $methods, bool $pa
* Checks if a URL matches the route pattern. Also parses named parameters in the URL.
*
* @param string $url Requested URL (original format, not URL decoded)
* @param bool $case_sensitive Case sensitive matching
* @param bool $caseSensitive Case sensitive matching
*
* @return bool Match status
*/
public function matchUrl(string $url, bool $case_sensitive = false): bool
public function matchUrl(string $url, bool $caseSensitive = false): bool
{
// Wildcard or exact match
if ($this->pattern === '*' || $this->pattern === $url) {
return true;
}

// if the last character of the incoming url is a slash, only allow one trailing slash, not multiple
if (substr($url, -2) === '//') {
// remove all trailing slashes, and then add one back.
$url = rtrim($url, '/') . '/';
}


Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the code above that actually fixes the problem.

$ids = [];
$last_char = substr($this->pattern, -1);

Expand Down Expand Up @@ -157,7 +164,7 @@ static function ($matches) use (&$ids) {
$regex .= $last_char === '/' ? '?' : '/?';

// Attempt to match route and named parameters
if (!preg_match('#^' . $regex . '(?:\?[\s\S]*)?$#' . (($case_sensitive) ? '' : 'i'), $url, $matches)) {
if (!preg_match('#^' . $regex . '(?:\?[\s\S]*)?$#' . (($caseSensitive) ? '' : 'i'), $url, $matches)) {
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions flight/net/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Router
/**
* Case sensitive matching.
*/
public bool $case_sensitive = false;
public bool $caseSensitive = false;

/**
* Mapped routes.
Expand Down Expand Up @@ -221,7 +221,7 @@ public function group(string $groupPrefix, callable $callback, array $groupMiddl
public function route(Request $request)
{
while ($route = $this->current()) {
$urlMatches = $route->matchUrl($request->url, $this->case_sensitive);
$urlMatches = $route->matchUrl($request->url, $this->caseSensitive);
$methodMatches = $route->matchMethod($request->method);
if ($urlMatches === true && $methodMatches === true) {
$this->executedRoute = $route;
Expand Down
4 changes: 2 additions & 2 deletions tests/EngineTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function getInitializedVar()
$engine->request()->url = '/someRoute';
$engine->start();

$this->assertFalse($engine->router()->case_sensitive);
$this->assertFalse($engine->router()->caseSensitive);
$this->assertTrue($engine->response()->content_length);
}

Expand All @@ -64,7 +64,7 @@ public function getInitializedVar()
// This is a necessary evil because of how the v2 output buffer works.
ob_end_clean();

$this->assertFalse($engine->router()->case_sensitive);
$this->assertFalse($engine->router()->caseSensitive);
$this->assertTrue($engine->response()->content_length);
}

Expand Down
18 changes: 17 additions & 1 deletion tests/RouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ public function testPathRouteTrailingSlash()
$this->check('OK');
}

public function testPathRouteWithUrlTrailingSlash()
{
$this->router->map('/path', [$this, 'ok']);
$this->request->url = '/path/';

$this->check('OK');
}

public function testGetRouteShortcut()
{
$this->router->get('/path', [$this, 'ok']);
Expand Down Expand Up @@ -455,7 +463,7 @@ public function testCaseSensitivity()
{
$this->router->map('/hello', [$this, 'ok']);
$this->request->url = '/HELLO';
$this->router->case_sensitive = true;
$this->router->caseSensitive = true;

$this->check('404');
}
Expand Down Expand Up @@ -752,4 +760,12 @@ public function testGetUrlByAliasWithGroupSimpleParams()

$this->assertEquals('/path1/123/abc', $url);
}

public function testStripMultipleSlashesFromUrlAndStillMatch()
{
$this->router->get('/', [ $this, 'ok' ]);
$this->request->url = '///';
$this->request->method = 'GET';
$this->check('OK');
}
}
2 changes: 1 addition & 1 deletion tests/commands/RouteCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ protected function createIndexFile()
Flight::delete('/delete', function () {});
Flight::put('/put', function () {});
Flight::patch('/patch', function () {})->addMiddleware('SomeMiddleware');
Flight::router()->case_sensitive = true;
Flight::router()->caseSensitive = true;

Flight::start();
PHP;
Expand Down
3 changes: 2 additions & 1 deletion tests/server/LayoutMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ public function before()
<li><a href="/protected">Protected path</a></li>
<li><a href="/template/templatevariable">Template path</a></li>
<li><a href="/querytestpath?test=1&variable2=uuid&variable3=tester">Query path</a></li>
<li><a href="/postpage">404 Not Found</a></li>
<li><a href="/badpagename">404 Not Found</a></li>
<li><a href="/postpage">405 Method Not Found</a></li>
<li><a href="{$final_route}">Mega group</a></li>
<li><a href="/error">Error</a></li>
<li><a href="/json">JSON</a></li>
Expand Down
Loading