Drop connection-supplied HTTP headers on cross-host redirects - #72165
Open
potiuk wants to merge 1 commit into
Open
Drop connection-supplied HTTP headers on cross-host redirects#72165potiuk wants to merge 1 commit into
potiuk wants to merge 1 commit into
Conversation
SameerMesiah97
left a comment
Contributor
There was a problem hiding this comment.
Left a comment. I believe that the existing tests i.e test_get, test_get_response_check, and test_sensor are now failing as they patch Session directly when this PR is now routing the connection through _ConnectionHeaderSession. I would adjust the patching for those tests accordingly to see if it fixes the failing tests.
potiuk
force-pushed
the
fix-http-conn-headers-cross-host-redirect
branch
from
August 27, 2026 20:09
56e80c8 to
f2ec50a
Compare
HttpHook applies every unrecognised key from a connection's extra field as a persistent session header (get_conn -> _configure_session_from_extra), and the hook documents that field as the place to put headers. requests removes only the Authorization header when a redirect crosses to a different host (Session.rebuild_auth), so a credential carried under any other header name -- X-API-Key and similar, which is what the documented pattern produces -- is replayed verbatim to the redirect target. Track which header names came from the connection and drop just those when requests' own should_strip_auth() says the hop is cross-origin. Headers passed explicitly by the caller are left alone, since the caller controls the request either way, and same-host redirects are unaffected. Delegating the decision to should_strip_auth also covers an https -> http downgrade on the same host.
potiuk
force-pushed
the
fix-http-conn-headers-cross-host-redirect
branch
from
August 28, 2026 14:38
f2ec50a to
51492ae
Compare
SameerMesiah97
approved these changes
Aug 29, 2026
1 task
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.
Why
HttpHook.get_connapplies every unrecognised key from a connection'sextrafield as a persistent session header:and the hook's own docstring documents that field as the place to put them — "headers can also be specified in the Extra field in json format".
requestsremoves only theAuthorizationheader when a redirect crosses to a different host (Session.rebuild_auth→should_strip_auth). A credential carried under any other header name —X-API-Key,X-Auth-Tokenand similar, which is exactly what the documentedextra-field pattern produces — stays on the session and is replayed verbatim to whatever host the redirect points at. Redirects are followed by default (session.max_redirects = DEFAULT_REDIRECT_LIMIT).What
_ConnectionHeaderSession, arequests.Sessionsubclass that records which header names came from the connection and drops only those whenshould_strip_auth()reports the redirect is cross-origin.get_conn, and populateconnection_header_keyswhere the extra-field headers are applied.get_conn.Delegating the decision to
requests' ownshould_strip_auth()means anhttps→httpdowngrade on the same host is covered too, without duplicating that logic.Compatibility
get_conn(headers=...),default_headers) are not affected — the caller controls the request either way.extra-derived headers are dropped, and only on a cross-origin hop.Testing
Verified locally against real
requests, all five cases passing: cross-host connection header dropped, same-host preserved, caller-supplied header preserved,Authorizationstill stripped byrequestsitself, and same-hosthttps→httpdowngrade stripped.The provider test suite could not be executed in my environment — a local editable-install issue unrelated to this change prevented
airflowfrom importing.ruff checkandruff format --checkare clean, andpy_compilepasses on both files, but CI needs to runproviders/httptests.🤖 Generated with Claude Code