Fix digest authentication for URLs with reserved characters#12436
Fix digest authentication for URLs with reserved characters#12436bdraco wants to merge 1 commit intoaio-libs:masterfrom
Conversation
The digest middleware computed the signature using yarl's path_qs, which returns the decoded form of the path and query string; aiohttp transmits the encoded form (raw_path_qs) on the wire, so servers signed a different request-target than the client and rejected the response with 401. Switch the digest A2 and the Authorization uri field to raw_path_qs so the signed value matches what is sent on the wire.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #12436 +/- ##
=======================================
Coverage 98.92% 98.92%
=======================================
Files 134 134
Lines 46750 46754 +4
Branches 2429 2429
=======================================
+ Hits 46248 46252 +4
Misses 373 373
Partials 129 129
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
Fixes HTTP Digest authentication failures when request paths or query strings contain percent-encoded reserved characters by ensuring the signed request-target matches what aiohttp actually sends on the wire.
Changes:
- Switch digest signing input (A2) and the
Authorizationheaderurifield fromURL.path_qs(decoded) toURL.raw_path_qs(wire-encoded). - Add parametrized unit tests covering encoded-vs-decoded mismatches for reserved characters and spaces.
- Add a Towncrier bugfix fragment documenting the behavior change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
aiohttp/client_middleware_digest_auth.py |
Uses raw_path_qs so digest signing matches the request-target actually transmitted. |
tests/test_client_middleware_digest_auth.py |
Adds coverage to ensure the uri in the digest header stays wire-encoded for tricky URLs. |
CHANGES/12436.bugfix.rst |
Documents the digest auth fix in release notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Merging this PR will not alter performance
Comparing Footnotes
|
|
I've been removing the 3.13 backports as I think we can release 3.14 in the next 2 weeks. Feel free to push a 3.13 release if it's urgent though. |
|
2 weeks is more than fine for this one. Thanks |
What do these changes do?
The digest middleware was computing the signature using yarl's
path_qs, which returns the decoded form of the path and query string; aiohttp sends the encoded form (raw_path_qs) on the wire, so the server signed a different request-target than the client and rejected the response with 401. This switches the digest A2 and the Authorizationurifield toraw_path_qsso the signed value matches what is sent on the wire.Are there changes in behavior for the user?
Digest auth now works against servers when the request path or query string contains percent-encoded reserved characters, for example
?action=9:\on Axis VAPIX endpoints. Requests that previously worked are unaffected since the encoded and decoded forms are identical when no reserved characters are present.Is it a substantial burden for the maintainers to support this?
No, the change is a single attribute swap on a yarl URL plus a parametrized test covering the encoded-vs-decoded mismatch.
Related issue number
Reported in Kane610/axis#774, where the downstream library had to ship its own digest-signing fallback to work around this.
Checklist
CONTRIBUTORS.txtCHANGES/foldername it
<issue_or_pr_num>.<type>.rst(e.g.588.bugfix.rst)if you don't have an issue number, change it to the pull request
number after creating the PR
.bugfix: A bug fix for something the maintainers deemed animproper undesired behavior that got corrected to match
pre-agreed expectations.
.feature: A new behavior, public APIs. That sort of stuff..deprecation: A declaration of future API removals and breakingchanges in behavior.
.breaking: When something public is removed in a breaking way.Could be deprecated in an earlier release.
.doc: Notable updates to the documentation structure or buildprocess.
.packaging: Notes for downstreams about unobvious side effectsand tooling. Changes in the test invocation considerations and
runtime assumptions.
.contrib: Stuff that affects the contributor experience. e.g.Running tests, building the docs, setting up the development
environment.
.misc: Changes that are hard to assign to any of the abovecategories.
Make sure to use full sentences with correct case and punctuation,
for example:
Use the past tense or the present tense a non-imperative mood,
referring to what's changed compared to the last released version
of this project.