-
Notifications
You must be signed in to change notification settings - Fork 436
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
contrib/dimfeld/httptreemux.v5: fix route and name for 30X redirects #2685
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
The httptreemux router has a redirect behaviour that is invoked when a request URL and matched route only differs in a trailing slash. The default behaviour is to respond with a 301 (moved permanently) to redirect the client to the exact path of the matched handler. We previously patched one of the two scenarios where this occurs in #2332. The changes in this commit addresses the other scenario for the 301 redirect, but also the 307 and 308 redirect options. Several tests were included to cover the various scenarios. We also standardize the resource name in the event that there is a trailing slash missmatch between request URL and matched handler. We always truncate the trailing slash in the resource name. Fixes #2663
Simplied the unnecessarily verbose function comments of isSupportedRedirectStatus and routerRedirectEnabled.
The new tests added to assert the redirect behaviour for the four redirect modes supported by the router had redundant test cases. These test cases were reduced down to three scenarios per redirect mode: 1. path not found 2. path with parameterized URL excluding a trailing slash 3. path with parameterized URL inclduing a trailing slash
When handling the supported redirect modes of the router we were incorrectly trimming trailing slashes from the route path matched by the router. The previous behaviour is restored where we return the exact path matched by the router.
github-actions
bot
added
the
apm:ecosystem
contrib/* related feature requests or bugs
label
May 8, 2024
BenchmarksBenchmark execution time: 2024-05-08 09:40:44 Comparing candidate commit b6005af in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 44 metrics, 0 unstable metrics. |
The old image has been unpublished in Docker Hub.
darccio
changed the title
contrib/dimfeld/httptreemux.v5: fix route and name for 30X redirects
[CI] contrib/dimfeld/httptreemux.v5: fix route and name for 30X redirects
May 8, 2024
6 tasks
rarguelloF
approved these changes
May 8, 2024
rarguelloF
approved these changes
May 8, 2024
darccio
changed the title
[CI] contrib/dimfeld/httptreemux.v5: fix route and name for 30X redirects
contrib/dimfeld/httptreemux.v5: fix route and name for 30X redirects
May 8, 2024
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.
The httptreemux router has a redirect behaviour that is invoked when
a request URL and matched route only differs in a trailing slash. The
default behaviour is to respond with a 301 (moved permanently) to
redirect the client to the exact path of the matched handler. We
previously patched one of the two scenarios where this occurs in #2332.
The changes in this commit addresses the other scenario for the 301
redirect, but also the 307 and 308 redirect options. Several tests
were included to cover the various scenarios.
We also standardize the resource name in the event that there is a
trailing slash missmatch between request URL and matched handler. We
always truncate the trailing slash in the resource name.
Fixes #2663<!--
CONTRIBUTING documentation.
-->
What does this PR do?
Motivation
Reviewer's Checklist
Unsure? Have a question? Request a review!