fix: preserve POST body across 3xx redirects (#1127, #1828) - #1848
Open
jacalata wants to merge 1 commit into
Open
fix: preserve POST body across 3xx redirects (#1127, #1828)#1848jacalata wants to merge 1 commit into
jacalata wants to merge 1 commit into
Conversation
`requests` follows 301/302/303 by converting POST to GET and dropping the
request body. Any TSC write hitting a server behind a redirect (users.add,
workbooks.publish, addusers, etc.) returned 405 Method Not Allowed because
the server saw a GET where it expected a POST.
Disable requests' auto-redirect and walk the chain manually in
Endpoint._make_request, keeping the original method and body across every
hop. Hop count bounded by session.max_redirects (default 30, same as
requests).
Also close two nearby gaps:
- Refuse HTTPS -> HTTP scheme downgrades. Silently following them would
send auth material over plaintext; no legitimate server behaviour
requires this. Raises RedirectError with the original and target URLs.
- Raise RedirectError (with URL, method, status code) when a 3xx response
has no Location header, replacing the bare KeyError('location') that
requests emits deep in its internals.
Sign-in retains its own single-hop 301 handler in auth_endpoint.py for
backwards compatibility; the new path is additive.
Test coverage: 8 new tests in test_redirect_handling.py covering POST body
preservation, multi-hop chains, relative Location headers, scheme
downgrade refusal, missing Location, and hop-cap enforcement. Existing
866-test suite unchanged.
Fixes #1127. Fixes #1828.
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
1 participant
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
requestsfollows 301/302/303 by converting POST to GET and dropping the request body. Any TSC write hitting a server behind a redirect (users.add,workbooks.publish,addusers, etc.) returned 405 Method Not Allowed because the server saw a GET where it expected a POST. See Follow multiple redirects instead of just one #1127 for the same request from another user.Endpoint._make_request, keeping the original method and body across every hop. Hop count bounded bysession.max_redirects(default 30, same as requests).Also closes two nearby gaps:
RedirectErrornaming both URLs. Fixes security: refuse HTTPS→HTTP scheme downgrade in sign-in redirect #1828.Locationheader. Replaces the bareKeyError('location')thatrequestsemits deep in its internals with aRedirectErrorthat names the URL, method, and status code.Sign-in retains its own single-hop 301 handler in
auth_endpoint.pyfor backwards compatibility; the new path is additive.Closes #1127. Closes #1828.
User-visible changes
Locationsame schemeLocation; caller sees the eventual 2xx/error from the real endpointsession.max_redirectshops; if exceeded, raisesRedirectErrorhttp://...RedirectErrorLocationKeyError: 'location'from deep insiderequestsRedirectErrornaming URL, method, status codeTest plan
test/test_redirect_handling.pycovering POST body preservation, multi-hop chains, relativeLocationheaders, scheme downgrade refusal, missingLocation, and hop-cap enforcement🤖 Generated with Claude Code