Make trailing slash rewrite use relative path

[jakezatecky]

The "Redirect Trailing Slash" RewriteRule does not use a relative path. Instead, it uses an absolute path to the web root. This causes an issue if you add RewriteBase to .htaccess.

To see this problem in action, consider the following `.htaccess:

RewriteBase /newbase

# Redirect Trailing Slashes...
RewriteRule ^(.*)/$ /$1 [L,R=301]

If we make a request to http://some-site.com/newbase/file/, we will be redirected to http://some-site.com/file, which is incorrect.

By changing the RewriteRule to RewriteRule ^(.*)/$ $1 [L,R=301], you now get redirected to the correct location: http://some-site.com/newbase/file

Granted, you could argue that since this issue would only come up if you edit the .htaccess file and add the RewriteBase rule, you should know to change the corresponding RewriteRule. However, if you know little about .htaccess files, you might spend an hour trying to figure out why you were redirected to the web root when an extra slash was added at the end.

It's important to note that this change does not appear adversely affect anything if we still use the web root.

[vlakoff]

I have the same problem (subdir, but no RewriteBase, never needed that), but your proposed fix doesn't work for me, got redirected to:

http://127.0.0.1/C:/UwAmp/www/awesomeproject/public/foo/bar

Yep, it's Windows, it's on local, I don't want to create virtual host for each project, etc. Exotic, I know.

Considering how dealing with .htaccess is always such a pain, I'd personally handle this at PHP level.

If anyone could suggest better .htaccess directives?

[vlakoff]

success 😃

# bonus, avoid infinite redirect in case a directory with the same name exists
RewriteCond %{REQUEST_FILENAME} !-d

# the ultimate no-trailing-slash technique
RewriteCond %{REQUEST_URI} (.+)/$
RewriteRule ^ %1 [L,R=301]

Also, working on %{REQUEST_URI} should be robuster. Last but not least, successfully tested with GET parameters.

jakezatecky, would you please give it a try too?

and for reference: 343c31e

[jakezatecky]

@vlakoff, I agree, .htaccess is a real pain. However, your solution worked phenomenally well! All of my tests yielded success and I could drop the annoying RewriteBase directive that I was previously using! Great job!

I would probably change the RewriteCond to the following to match with what's already in .htaccess:

RewriteCond %{REQUEST_URI} (.+)/$

My solution was flawed, as you noted. The issue you experienced is mitigated if you use RewriteBase /awesomeproject/public, but I really like your solution much better. I did not realize that my solution actually required RewriteBase! I did not catch this when I was testing because I had several cached redirects in my browser due to the 301 code being thrown around.

[vlakoff]

Glad to hear :)

Please add the directory condition in your PR, I think it's a must.

Then, it is important to use (.+)/$ and not (.*)/$, otherwise very bad things would happen when accessing the root /.

Summary:

    # Redirect Trailing Slashes...
    RewriteCond %{REQUEST_FILENAME} !-d
    RewriteCond %{REQUEST_URI} (.+)/$
    RewriteRule ^ %1 [L,R=301]

Side note: for debugging these redirects, I have to use Chrome and clear browser cache every time. Firefox keeps giving me stale results.

[jakezatecky]

Makes sense. For what it's worth, I was successfully able to test both Firefox and Chrome while clearing the cache/history.

[vlakoff]

Looks like it's also fine with Firefox now; it's just I remember having had trouble with it previously.

@taylorotwell If you agree, the code is ready to merge. This time, promised :) Summary: Fix trailing slash redirect when app located in a subdirectory.

[stuarthannig]

👍

[vlakoff]

Why not?

[franzliedke]

Probably because Taylor has said before Laravel should be run from a domain root.

It would be lovely if this could be reconsidered.

[vlakoff]

This has revealed itself to be quite popular.

Considering I have developed for clear reference my answer on SO, could this issue be reconsidered?

(Ah, and… mine is robuster than yours was :p)

[vlakoff]

This has been added to Laravel, finally: #4344 🙌