Skip to content
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

Improve redirect handling #10981

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

elsloo
Copy link
Contributor

@elsloo elsloo commented Jan 9, 2024

  • Do not permit a 302 to be cached if a redirect is in progress.
  • Could be more permissive if we remove the check for HTTP_STATUS_MOVED_TEMPORARILY in HttpTransact::is_response_cacheable()
  • Refactor HttpSM::is_redirect_required() to make it easier to call when state is incomplete
  • May address issue ATS 9.2.x and origin 301/302 chase redirections #10955

@masaori335 masaori335 added this to the 10.0.0 milestone Jan 10, 2024
* Do not permit a 302 to be cached if a redirect is in progress.

* Could be more permissive if we remove the check for `HTTP_STATUS_MOVED_TEMPORARILY` in `HttpTransact::is_response_cacheable()`

* Refactor HttpSM::is_redirect_required() to make it easier to call when state is incomplete.
@elsloo elsloo force-pushed the fix_cached_redirect_in_progress branch from 88659cc to 37b6fd0 Compare January 10, 2024 04:34
@elsloo elsloo marked this pull request as draft January 17, 2024 18:06
@elsloo
Copy link
Contributor Author

elsloo commented Jan 17, 2024

Changing this PR to a draft until I complete testing and add the AuTests to this PR with a description of what it solves. Without this PR we have corner cases that can lead to a 302 being cached unintentionally, which then leads to secondary problems due to the 302 itself being possibly stuck in cache and served to other caches.

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. Marking it stale to flag it for further consideration by the community.

@github-actions github-actions bot added the Stale label Apr 17, 2024
@github-actions github-actions bot closed this Apr 24, 2024
@zwoop zwoop removed this from the 10.0.0 milestone May 13, 2024
@bryancall bryancall reopened this Oct 7, 2024
@bryancall bryancall marked this pull request as ready for review October 7, 2024 22:26
@bryancall bryancall marked this pull request as draft October 7, 2024 22:26
@bryancall
Copy link
Contributor

[approve ci]

@github-actions github-actions bot removed the Stale label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants