-
Notifications
You must be signed in to change notification settings - Fork 53
Infinite loop redirecting fix #262
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @DovydasGusarovas, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly improves the stability and reliability of the 404 handler by addressing potential infinite redirect loops. My changes introduce explicit checks to prevent redirects that would lead back to the referring URL, and also ensure that only one redirect attempt is made per request, making the system more resilient to complex redirection scenarios and misconfigurations.
Highlights
- Infinite Loop Prevention (Referrer-based): I've implemented a crucial safeguard within the
HandleRequest
method to prevent infinite redirect loops. Specifically, if a determined redirect's target URL (NewUrl
) matches the referrer URL of the current request, the redirect will now be blocked. This addresses edge cases where a 404 handler could redirect back to the source, causing an endless cycle of 404s and redirects. - Single Request Redirect Limit: To further enhance robustness, I've added a mechanism in the
Handle
method to ensure that a redirect is only attempted once per HTTP request. A newRedirectedOnceKey
is used in theHttpContext.Items
collection to track if a redirect has already been processed for the current request, preventing redundant or recursive redirect attempts within the same request lifecycle. - New Unit Test Coverage: A new unit test,
HandleRequest_returns_false_when_redirect_matches_referrer()
, has been added to explicitly verify the new referrer-based loop prevention logic. This ensures the intended behavior is correctly implemented and protected against future regressions.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively fixes a potential infinite redirect loop by adding checks to prevent redirecting to the same URL or back to the referrer. It also introduces a safeguard against re-entrant calls to the handler within the same request. The changes are logical and include a new unit test to cover the fix. My review includes a few suggestions to improve logging clarity and code style.
if (canHandleRedirect && newUrl.State == (int)RedirectState.Saved) | ||
{ | ||
LogDebug("Handled saved URL", context); | ||
LogDebug("Handled saved URL", context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jevgenijsp are you up for some code review? Thanks in advance |
Background
In certain edge cases, the 404handler was causing an infinite redirect loop resulting in long, repeated paths like:
/om-brand/hvorfor-vaelge-os/hvorfor-vaelge-os/hvorfor-vaelge-os/.../rabataftaler/fdm/foo
This occurred when:
A redirect exists for a "not found" URL (e.g.,
/brand/.../foo
)The redirect target URL (
NewUrl
) matched the referrer of the current requestThe logic did not properly prevent redirection back to the referrer, which triggered another
404
, causing a loop✅ What's been fixed
The logic in
HandleRequest()
already guards against redirect loops where:NewUrl == urlNotFound.PathAndQuery
✅This PR adds an additional safeguard:
Block redirect if
NewUrl == referrer.PathAndQuery
❗This prevents redirecting back to the source, which would repeat the same failure and retry cycle indefinitely.
🔬 What was added
A new unit test:
HandleRequest_returns_false_when_redirect_matches_referrer()
Ensures
HandleRequest()
returns false if the redirect would loop back to the referring URLThe logic to block referrer-based loops already existed, this test explicitly verifies that behavior and protects it against regression
🔄 Why it matters
Prevents unnecessary
404
redirects and infinite loopsProtects end users and search crawlers from broken navigation
Saves server resources and ensures cleaner error handling
Makes redirect logic more robust in content-rich environments where URL overlaps can happen