Remove trailing slashes from file URLs in web::server class

Remove trailing slashes from file URLs in web::server class

[Wladimir Palant, 2017-06-26 22:20:47]

Is it a good idea to have this land without an issue? It's not a trivial change.

https://codereview.adblockplus.org/29474653/diff/29474654/modules/web/templat...
File modules/web/templates/site.conf.erb (right):

https://codereview.adblockplus.org/29474653/diff/29474654/modules/web/templat...
modules/web/templates/site.conf.erb:52: if (!-d "$document_root/$uri")
You probably want to do this only for actual files, not just for any input that
might be a directory.

https://codereview.adblockplus.org/29474653/diff/29474654/modules/web/templat...
modules/web/templates/site.conf.erb:54: rewrite ^(/.*)/$ $1 redirect;
In the case we are talking about (something that was previously a directory
became a regular page) this will cause a redirect loop because the URL without
the slash has a permanent redirect stored in local cache. An internal redirect
would do but is ugly IMHO, maybe redirect with a "?no_redirect" added to the
URL? The pages specify a canonical URL, so we shouldn't have this URL mess up
search results.

[mathias, 2017-06-27 10:04:01]

On 2017/06/26 22:20:47, Wladimir Palant wrote:
> Is it a good idea to have this land without an issue? It's not a trivial
change.

No idea what a ticket could improve here. The intended commit message is pretty
straightforward, and the patch-set is as well. Don't get me wrong, I am
definitely not against having tickets, I just wonder how subsequently creating
one now has any benefit. Isn't there a ticket for the issues which lead to this
one?

https://codereview.adblockplus.org/29474653/diff/29474654/modules/web/templat...
File modules/web/templates/site.conf.erb (right):

https://codereview.adblockplus.org/29474653/diff/29474654/modules/web/templat...
modules/web/templates/site.conf.erb:52: if (!-d "$document_root/$uri")
On 2017/06/26 22:20:47, Wladimir Palant wrote:
> You probably want to do this only for actual files, not just for any input
that might [not] be a directory.

In fact i wanted to explicitly express that this is done "for anything but
directories", in contrast to Nginx behavior for serving directories. But you're
right: The current version would also match absent resources, which in turn
could lead to some overhead with invalid requests and issues with i.e. FCGI
backends.

https://codereview.adblockplus.org/29474653/diff/29474654/modules/web/templat...
modules/web/templates/site.conf.erb:54: rewrite ^(/.*)/$ $1 redirect;
On 2017/06/26 22:20:47, Wladimir Palant wrote:
> [...] maybe redirect with a "?no_redirect" added to the URL?

Acknowledged.