-
Notifications
You must be signed in to change notification settings - Fork 11.4k
[Proposal] Remove the trailing slash in URL #58
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
Comments
👍 |
I'm sure github does that before hitting the application. |
We've already had this discussion, it's wanted behavior since there are some advantages. |
@zimt28 I don’t see the point, could you add a link to the discussion? The article you are mentioning is about returning 200 or 301/302 redirects to handle trailing slashes, not 404. More precisely, it seems Google is ok with 200 on both, even without
And recommends to pick one and redirect the other one:
|
I actually wanted to do that but the discussion took place on the old illuminate/router or something which has been deleted in between. |
@bpierre The problem is that Laravel 4 currently only works with the variant that you specify in your routes. If you don't use a slash there, the variant with the slash will return a 404 error message. |
I was actually surprised when I noticed laravel didn't do this already, it just seems logical to me that it should match fine with or without the trailing slash. |
@franzliedke Yes, and with a slash, that’s the variant without slash which returns a 404. I want both :-) I tried to add Route::get('admin{slash?}', function($slash)
{
// (╯°⁔°)╯ ︵ ┻━┻
})
->where('slash', '\/'); Plus, this still requires to handle the redirect. |
+1 on having some way to do this. |
I agree entirely, hackable URLs are brilliant. I actually raised this issue earlier, but @taylorotwell closed it saying "Just don't use trailing slashes." |
@BenJenkinson Thanks for the link. As you said, it’s not our fault, it’s our users, and we have to guide them to the no-trailing-slash enlightenment :-) |
That's super annoying during development, though. I certainly don't remember to delete the slash when I manually go up in the hierarchy by deleting the last URL segment. |
It is both annoying and non-obvious, as demonstrated by the identical issues raised by myself earlier, @bpierre here and @DPr00f over on laravel/laravel Issue #1596. I also found this earlier laravel/laravel Issue #1368 where they point out that redirecting will sometimes cause problems with form submission; if the URL used for the form |
@bpierre , @frankmayer, @zimt28 , @billmn Using regex in L4 makes things so much more powerful. Let's not fix what isn't broken. Route::get('{any}', function($url){
return Redirect::to(mb_substr($url, 0, -1), 301);
})->where('any', '(.*)\/$'); |
@wishfoundry, yes that would be easy, but for people just starting off with L4 I see a lot of feature and help requests. Maybe it would be good to have this in the docs? |
@wishfoundry Your method looks like it can do the desired need to handle the kind of url issue we have here, but in my opinion it is little cumbersome to do the redirection on each route. |
Although the greatest flexibility would be if Laravel would use the routes to decide what to redirect, but personally I'm ok with choosing one and redirecting in the htaccess.
|
@tlgreg With your htaccess I'm unable to load assets from /public/packages/vendor/... |
@tlgreg I may be wrong, I am far from being an Apache guru:
|
@billmn Sorry, missed the non-existing file condition for the index. Actually the file condition is not that important for the redirects as they shouldn't end with a slash anyway. I updated the post above. @bpierre Yes, this is a bit quick and dirty in a way that it redirects everything.
This way for trailing slashes with POST/PUT/DELETE, etc a 404 error will be returned by Laravel. I'm not an Apache guru either, this was just my quick take on it and needs testing. :) |
One trailing slash is now allowed. |
With the following URLs:
I can’t “hack” the URL by removing the
users
part of the second one, a 404 will be triggered because of the trailing slash:I think that “hackable URLs” are a nice way to interact with an application.
Proposal
The
Router
class could handle this: if a route is not matched, but the path ends with a slash, then try again to match the route after removing the trailing slash.If the route exists, then redirect to the correct route, without trailing slash. Or maybe don’t even redirect, like GitHub does (it could be a setting):
Router->dispatch()
could handle this, or maybeRouter->handleRoutingException()
.The text was updated successfully, but these errors were encountered: