fix: check callable before tuple in prepare_auth#7486
Closed
qizwiz wants to merge 1 commit into
Closed
Conversation
When auth is both callable and a 2-element tuple subclass (e.g. a namedtuple-based AuthBase subclass), the previous code took the isinstance(auth, tuple) branch first and silently constructed HTTPBasicAuth(*auth) instead of invoking the object's __call__. This meant the custom auth handler was never called and its fields were extracted as Basic Auth credentials. Flip the check order so callable(auth) is tested first. Plain (username, password) tuples are unaffected: HTTPBasicAuth is itself callable, but a bare tuple is not, so it still reaches the isinstance branch correctly. Closes the existing TODO comment: "can be fixed by flipping the conditionals"
Member
|
Hi @qizwiz, thanks for the PR. The TODO was a note for ourselves and intentionally left for 2.34.x to keep the surface area of the change smaller. We already have a patch prepared for when we do the flip. I'll close this out as it won't be needed. |
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
prepare_authcurrently checksisinstance(auth, tuple)beforecallable(auth). Any auth object that is both a 2-element tuple subclass and callable — for example anAuthBasesubclass that also inherits from a 2-fieldnamedtuple— silently takes the tuple branch. Its__call__is never invoked; instead its fields are extracted and passed toHTTPBasicAuth, bypassing the custom handler entirely.This PR closes the existing TODO:
# TODO: can be fixed by flipping the conditionalsWhat changes
src/requests/models.py— flip the conditional order:tests/test_requests.py— regression test: a callable namedtuple auth handler must use__call__, not be downgraded toHTTPBasicAuth.Why this is safe
Plain
(username, password)tuples are unaffected: a baretupleis not callable, so it still reaches theisinstancebranch.HTTPBasicAuthis itself callable, but it is not a bare tuple, so it routes throughcallablecorrectly as before.Proof of bug
Reproduced against requests 2.32.5:
The custom
__call__was never invoked. The bearer token was silently encoded as a BasicAuth username.This was found via Z3 formal verification of an
orderingcontract onprepare_auth(callable check must precede tuple check).