Framework: Bug in routing

[mpskovvang]

• Laravel Version: 5.4.19
• PHP Version: 7.1

Description:

This issue is a bit related to the many previous discussions about leading/trailing slashes but differs. The bug even affects laravel.com. If the developer doesn't pay attention he might be a victim of duplicate content.

It's best explained by an example:

Route::get('foo', function () {
    return "Hello World";
});

Because the matching does care about leading/trailing slashes the following URLs works:

/foo
/foo/

The real issue is that the following example also works:

/////foo/////

So you can navigate to e.g. https://laravel.com////docs/5.4/routing/////

I would except the router to be more strict. I know an exact match implementation will be a breaking change so I suggest an opt-in solution by a config setting like router.strict=true|false.

By modifying a single line in the prefix method in IlluminateRoutingRouter and a single line in the matches method in IlluminateRoutingMatchingUriValidator the bug is solved.

prefix method: return $this->getLastGroupPrefix() . $uri;

matches method: $path = $request->getPathInfo();

It also solves the long discussion about trailing slashes as it will be the developers choice per route:
Route::get('/foo', ...) will only be matched by /foo
Route::get('/foo/', ...) will only be matched by /foo/

_My suggestions aren't really tested but just some quick thoughts to start a discussion._

[fernandobandeira]

Having to create two routes to match /foo and /foo/ seems a bit too much for me...
IMO we could do the modifications you suggested and maybe create a check, if the url ends with / then create a 301 redirect to the same url without the leading /.

We should probably provide a way to disable this redirect however IMO that's the most common usecase...

[mpskovvang]

My intention was not to support both /foo and /foo/ but just one of them. It was just an example to show how the new matching method would work.

I understand that some people might want to match /foo and /foo/ from a single route which I why I suggest a strict and non-strict matching implementation.

Personally, I would except a route to match exactly that I typed.

[Kyslik]

@fernandobandeira with redirects you could end up with TOO_MANY_REDIRECTS easily, make named route to one with trailing slash and you are in trouble (of course its up to programmer to make things right and this would hopefully never leave local development).

I don't see anything wrong with /////foo/////, users that want to tinker with URL will still tinker with it, maybe you can detect such thing and throttle it to "scare tinkers away" (I know dumb but it might work in some cases)

[mpskovvang]

I solved my issues with a simple middleware with parameters specifying how trailing and duplicated slashes should be handled. Works much better and easier implemented!

I needed another solution as my suggested modifications to the router breaks route parameters.

[mpskovvang]

Currently, I configure how slashes should be handled for each route group.

It would be nice if my middleware could choose the action based on the typed route. If I typed /foo/ then it new it should force the trailing slash.

But unfortunately, the router trims the slashes when adding the route which means my middleware doesn't know what I original typed. I could use custom attributes but that seems stupid typing the same twice.

I suggest a property on the route class and a method returning the original name - before trimming and compiling the route.

What are your thoughts?

[Kyslik]

I think your use-case (or whatever we are talking about) is really out of the reality.

If user is changing URL on purpose, you can not change that, just ignore it.

[mpskovvang]

My concerns are SEO related. I'm not sure but these different URLs for the same content might be seen as Duplicate Content.

One with evil intentions might index these different URLs to harm your entire business. Personally, I do not want to take these risks.

It might sound as a user-specific case, but I bet it is not. Unfortunately, I can not prove the risk, but better safe than sorry.

[ImadJomaa]

Given that that's your concern, you can rest assured that many major websites (including this one) have the same pattern matching w/o any noticeable consequences. It would take a lot of effort on someone's part to try and ding your ranking with this, should it have any merit. I think it's safe to say, there are easier ways to hurt your business.

[bbashy]

Personally, I do not want to take these risks.

Then don't. Include your own routing or verify the segments don't have multiple slashes etc and you win.

This shouldn't be messed with at a framework level.

[nathiboat]

I am curios how u done setting on https://laravel.com////docs/5.4/routing///// I want my web to work the same result as this too.