[7.x] Fix double slashes matching in UriValidator (fix inconsistencies between cached and none cached routes)

[BafS]

Sorry to open a new PR but the other one was closed, I think there were some misunderstanding.
The goal of this PR is to not match routes when we have double slashes and also fix the misleading 405 error with cached route collection when we have a double slash.

Before the PR

URL	Not cached	Cached
http://127.0.0.1/foo	foo	foo
http://127.0.0.1/foo/bar	bar	bar
http://127.0.0.1//foo ⚠️	foo	405
http://127.0.0.1//foo/bar ⚠️	bar	405
http://127.0.0.1/foo//bar	404	404
http://127.0.0.1//foo/bar	404	404

With this PR

URL	Not cached	Cached
http://127.0.0.1/foo	foo	foo
http://127.0.0.1/foo/bar	bar	bar
http://127.0.0.1//foo	404	404
http://127.0.0.1//foo/bar	404	404
http://127.0.0.1/foo//bar	404	404
http://127.0.0.1//foo/bar	404	404

The problem is that UriValidator uses $request->path() which use the trimmed path but CompiledRouteCollection uses:

    $parts = explode('?', $request->server->get('REQUEST_URI'), 2);

    $trimmedRequest->server->set(
        'REQUEST_URI', rtrim($parts[0], '/').(isset($parts[1]) ? '?'.$parts[1] : '')
    );

with rtrim, which creates the discrepancy.

What do you think ? Please let me know if you want me to fix it differently, I just want to fix this inconsistency.

[danilopinotti]

Thoughts about sanitize route before matching?

[GrahamCampbell]

Thoughts about sanitize route before matching?

Why? We want routing to be as fast as possible.

[GrahamCampbell]

And what did you mean exactly be sanitizing?

[GrahamCampbell]

@BafS could you add a couple of new tests please?

[danilopinotti]

Why? We want routing to be as fast as possible.
And what did you mean exactly be sanitizing?

Maybe 'sanitize' is the wrong word.
What I meant was removing duplicate slashes in the route before matching.

I saw this behavior in some sites (github, by example)

[BafS]

@danilopinotti You can do that with your server, I don't think it's Laravel's job to do it

@GrahamCampbell I added a test.

I think it would be good to run the tests in CompiledRouteCollectionTest against the cached and none cached routes to be sure we stay consistent but I didn't want to add too much refactoring here.

[no0by5]

This PR is actually breaking my application. I provide a calendar through my application, and for some reason the frontend returned an URL with a double slash for the calendar. So this URL is now used by many users to import the calendar, but since this was merged, these imports are no longer working. Is there a way to work around this? Adding a route with '//' is filtered out.

[driesvints]

Urls with // aren't valid. There's no workaround for this.

[BafS]

You can workaround that with your server, by using url rewrite, but it's not a good practice