Fixes redirect loop in serve-static. #761
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When using serve-static with static_embed and defaults:
Router::with_path("<**>").get(static_embed::<Assets>().defaults("index.html"));
with a non-root folder with a default file ("index.html"):
├─ index.html
└─ foo/
└─ index.html
trying to GET the "/foo" path results in a redirect loop, with salvo
adding an extra "/" after each iteration: "/foo//////////////////".
This is because of the code in embed.rs that tries to redirect from
"/foo" to "/foo/":
135 | if embedded_file.is_some() && !req_path.ends_with('/') && !req_path.is_empty() {
136 | redirect_to_dir_url(req.uri(), res);
137 | return;
138 | }
where req_path was built with:
123 | let req_path = format_url_path_safely(&req_path);
but `format_url_path_safely()` always strips the final slash(es), which
means the `!req_path.ends_with('/')` check always succeeds, and
`redirect_to_dir_url()` is called on `req.uri()` (not `req_path`), which
still has all its slashes. `redirect_to_dir_url()` then unconditionally
adds another slash, and the loop repeats.
This commit breaks the loop by having `format_url_path_safely()` keep
one final slash if it was already present, so the check on line 135 now
works as expected.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using serve-static with static_embed and defaults:
Router::with_path("<**>").get(static_embed::().defaults("index.html"));
with a non-root folder with a default file ("index.html"):
├─ index.html
└─ foo/
└─ index.html
trying to GET the "/foo" path results in a redirect loop, with salvo adding an extra "/" after each iteration: "/foo//////////////////".
This is because of the code in embed.rs that tries to redirect from "/foo" to "/foo/":
135 | if embedded_file.is_some() && !req_path.ends_with('/') && !req_path.is_empty() {
136 | redirect_to_dir_url(req.uri(), res);
137 | return;
138 | }
where req_path was built with:
123 | let req_path = format_url_path_safely(&req_path);
but
format_url_path_safely()always strips the final slash(es), which means the!req_path.ends_with('/')check always succeeds, andredirect_to_dir_url()is called onreq.uri()(notreq_path), which still has all its slashes.redirect_to_dir_url()then unconditionally adds another slash, and the loop repeats.This commit breaks the loop by having
format_url_path_safely()keep one final slash if it was already present, so the check on line 135 now works as expected.