Add TrailingSlash::MergeOnly behavior

[SuperHacker-liuan]

Behavior of NormalizePath in actix-web 2.0 would keep the trailing slash's existance as it is,
which is different from that in 3.0.

This patch add a new option called MergeOnly in TrailingSlash, which only merge the trailing
slashes into one if there exist. This option makes NormalizePath able to be compatible with
actix-web 2.0 and makes it eaiser for users who want to migrate from 2.0 to 3.0.

This will close #1694.

PR Type

Bug Fix

PR Checklist

Check your PR fulfills the following:

• Tests for the changes have been added / updated.
• Documentation comments have been added / updated.
• A changelog entry has been made for the appropriate packages.
• Format code with the latest stable rustfmt

Overview

New option in TrailingSlash will makes it easy for developers who want to migrate
from 1.0 / 2.0 to 3.0, because the behavior of this option is compatible with legacy versions.

No breaking changes.

close #1694.

[codecov-commenter]

Codecov Report

Merging #1695 into master will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1695      +/-   ##
==========================================
+ Coverage   53.32%   53.41%   +0.09%     
==========================================
  Files         125      125              
  Lines       11918    11942      +24     
==========================================
+ Hits         6355     6379      +24     
  Misses       5563     5563              

Impacted Files	Coverage Δ
src/middleware/normalize.rs	98.61% <100.00%> (+0.27%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c53e946...6ff4da7. Read the comment docs.

[Neopallium]

Looks good.

[robjtede]

I think it should be discussed whether this should be considered a permanent option or just for migration from v2 (and then removed with the next breaking changes).

I personally can't see the use case for wanting the distinction and consider it a foot gun to allow it in this middleware. In my view, either all your routes are defined with trailing slashes or they aren't.

[SuperHacker-liuan]

Browsers are affected by the trailing slash policies when they are trying to find resource via relative paths.
When browsers trying to request a resource via relative path, TrailingSlash::Always & TrailingSlash::Trim will cause a resource missing in some situation.

Example 1:

I've got a site provides /app/ which will response a page A, and /app/style.css which will response a style sheet file being used for /app/. In page A, there will be a tag link to the css as following.

<link href="style.css" type="text/css" rel="stylesheet"/>

No matter how the route I defined, users' access to /app & /app/ will get difference. because browsers which trying to request /app will request /style.css which is in fact not exist in my routes.

To fix the problem above, I'd have to write the routes which response /app to HttpResponse::SeeOther().header(header::LOCATION, "app/").finish() to solve this problem.

Example 2:

Consider I want /app and /app/ both redirect to /app/index.

When I use TrailingSlash::Trim, they will both use the /app route, which will reponse a HTTP 303 to app/index. Browsers which are trying to request /app/ will be redirected to /app/app/index which is in fact not exist.

When I use TrailingSlash::Always, they will both use the /app/ route, which will response a HTTP 303 to index. Browsers which are trying to request /app will be redirected to index which may be another page.

Only when I use TrailingSlash::MergeOnly, and redirect /app & /app/ to different locations, users' browsers are able to perform the correct action.

[robjtede]

It seems most of your problems could be solved, client-side, by not relying so heavily on relative paths. That being said, the case of redirection and relative path resolving in browsers is compelling to try solving anyway.

From the perspective of an actix app serving markup, an alternate way to solve this would be an option within this middleware to redirect to the normalized path rather than transparently normalizing and serving that resource. I think this idea has been mentioned before. Prior art for this approach includes Apache and Nginx rewrite rule flags.

[SuperHacker-liuan]

It seems most of your problems could be solved, client-side, by not relying so heavily on relative paths. That being said, the case of redirection and relative path resolving in browsers is compelling to try solving anyway.

From the perspective of an actix app serving markup, an alternate way to solve this would be an option within this middleware to redirect to the normalized path rather than transparently normalizing and serving that resource. I think this idea has been mentioned before. Prior art for this approach includes Apache and Nginx rewrite rule flags.

Well, I'm afraid that w/ this approach, I'll not able to deploy actix-web sites directly to 80/tcp or 443/tcp, but have to set a reverse proxy for it.

[robjtede]

Well, I'm afraid that w/ this approach, I'll not able to deploy actix-web sites directly to 80/tcp or 443/tcp, but have to set a reverse proxy for it.

I want to try to fully understand your needs here so we can come up with the best solution.

It occurs to me that this is a solved problem on the reverse proxy/static page servers like apache and nginx where they have chosen to use the rewrite rules system that can optionally redirect to the normalized page. As far as I can tell, this solves the same problem and is worth considering as an alternative.

You've even suggested this will solve the problem:

To fix the problem above, I'd have to write the routes which response /app to HttpResponse::SeeOther().header(header::LOCATION, "app/").finish() to solve this problem.

...so providing a built-in way to redirect to normalized paths would work and be more familiar to folks coming from apache/nginx.

Example 1: using a relative link here is dangerous anyway, using href="/app/style.css" would be better and serves /app and /app/ the same file.

Example 2: same idea, you want to redirect to /app/index, so use that as the redirect path instead of trying to use relative urls.

There's lots of things to consider when adding a feature like this so I'd like to be convinced this is solving the right problem.

[SuperHacker-liuan]

Well, what I'm facing is a little troublesome.

0x0. Working background

I'm working in an company w/ tens of branches and a huge intranet. The company I'm working for is not a company w/ many professioned server operators, hence many employees are not able to write any config files. In fact, it is a company in traditional industries. As a part of them but not a full-time developer, I often develop some small HTTP services to reduce my pressure. Sometimes, employees in other branches would tell me that they want to deploy my service in theirs branches, bad sadly, they are neither developers nor professioned server operators. In past years, I'd like to provide my HTTP services executables to them but they are unable to prepare the executing environment themselves. (i.e. NodeJS, docker & etc.) What the worse is I'm also wasn't able to access their server becasue the cyber departments in those branches would never allow theirs firewall policy request, in which will aceept the connection from me.
Therefore I'll have to design my service whose releases will be deployed as easy as possible .

We can consider most of them are only able copy my executable to their server's /home/user dir with Filezilla and then execute ./app --port=12345 --db=10.4.5.6:3306 --daemon to start it, and finally request their cyber department to set the load balance at their F5 Big-IP load balancer's graphical interface.

In order to make the deployment as easy as possible, I have use the rust-embed to store the statics folder into the release executable, and implement a trait which will make actix-web able to load static files from struct StaticEmbed directly and automatically implement a Responder for it.

0x2. Why I used to face the problems like example 1 & 2

In some branches which are financial challenged or far away from big metropolics, they would not able to hire a professional server operator to maintaince their servers. Services which are not provided by parent company have to be deployed by employees who are not familiar with servers. Those branches would provide a hardware load balancer like F5 Big-IP, but cannot provide independent hostnames, because DNS server in our intranet is controlled by parent company, and to applicate a new IP is also a painful work for them. Hence they often use the same hostname but different access folders for different service. Of course I know that's legacy way and not the best practise, but I cannot affect their choice.

In my experience, I found that use the relative path is the best way to avoid the troubles for them, because there were no need to config the root path when deploying the app, and compatible with any URL prefix. E.g. In branch A, they may map http://foo.A.intranet/ to /foo/. In branch B, they may map http://inner.B.intranet/foo to /foo. And in branch C, they may map https://service.C.intranet/inner/foo to /foo. If I use absolute path, I'll have to face problem that if the deployers want to perform a config-free deployment or an one-stop deployment, only the deployment in branch B is rendered well.

I believe that not only me but also other friends may face the similar problem in traditional industries, especially in some developing countries. So I hope this PR will be accept, otherwise I'll have to write a independent external crate for similar requirements😭

[robjtede]

Thank you for the detailed explanation; it is a compelling reason to add it.

[SuperHacker-liuan]

Thank you for your suggestion @robjtede . I have appended your suggestions to the new commit.