Proxito: check forced redirects before system redirects#12903
Draft
ericholscher wants to merge 4 commits intomainfrom
Draft
Proxito: check forced redirects before system redirects#12903ericholscher wants to merge 4 commits intomainfrom
ericholscher wants to merge 4 commits intomainfrom
Conversation
Forced exact redirects are now checked before system redirects (e.g., / -> /en/latest/) so users can redirect entire projects to another domain with a single redirect rule. Fixes #10314
Clarify that forced exact redirects are checked before built-in redirects, so a single rule on /* is sufficient to redirect all traffic including the root URL, translations, and subprojects.
Add tests verifying that forced wildcard redirects on the main project catch translation paths, translation root, subproject paths, and subproject root before system redirects. Also verify non-wildcard redirects on the main project do not leak into subproject paths.
When forced redirects fire before URL resolution (the new early check), set request.path_project_slug and attempt to resolve the path to get version info for proper CDN cache control and cache tag headers. If resolution fails (e.g., "/" before system redirect), default to caching the redirect response since the target URL will handle authz.
This file contains hidden or 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
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.
Summary
Forced exact redirects are now checked before system redirects (e.g.,
/→/en/latest/) inserve_path(), so users can redirect entire projects to another domain with a single forced redirect rule.Previously, a request to
/would always be system-redirected to/en/latest/before any user-defined redirects were checked. This meant redirecting an entire project to another domain required creating forced redirects for every system redirect path (/,/en/,/en/latest/, etc.) instead of a single/*rule.How it works
serve_path(), beforeunresolve_path()is called/*on the main project catches all traffic including translations (/es/...) and subprojects (/projects/subproject/...)Changes
readthedocs/proxito/views/serve.py— Added early forced redirect check before system redirectsreadthedocs/proxito/tests/test_old_redirects.py— AddedUserForcedRedirectBeforeSystemRedirectTestswith 13 tests; updatedtest_exact_redirect_with_wildcardto expect 302 instead of 404docs/user/user-defined-redirects.rst— Documented forced redirect ordering in "Limitations and observations" and enhanced the "Migrating to another domain" exampleCloses #10314
Test plan
UserForcedRedirectBeforeSystemRedirectTestscovering:/to external domain/*from root and subpaths/es/latest/...) and translation root (/es/)/projects/subproject/en/latest/...) and subproject roottest_old_redirects.pypasstest_redirects.pypassMade by AI