-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Ensure cookies are still parsed after a malformed cookie #11724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Browsers and nodejs cookie parser are a lot more permissive so we should handle similar cases. fixes #11632
CodSpeed Performance ReportMerging #11724 will not alter performanceComparing Summary
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11724 +/- ##
========================================
Coverage 98.73% 98.74%
========================================
Files 127 127
Lines 43558 43661 +103
Branches 2320 2323 +3
========================================
+ Hits 43008 43111 +103
+ Misses 390 389 -1
- Partials 160 161 +1
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the cookie parsing logic to handle malformed cookies more gracefully, ensuring that subsequent valid cookies are still parsed when encountering cookies that fail regex validation (such as Google's g_state cookie with unescaped quotes). This aligns the behavior more closely with how browsers and Node.js handle cookie parsing.
Key Changes
- Added fallback parsing logic when regex matching fails, attempting simple key=value extraction
- Enhanced test coverage with 7 new test cases covering large values, multiple equals signs, whitespace handling, and edge cases in the fallback parser
- Removed xfail marker from previously failing
test_parse_cookie_gstate_headertest
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| aiohttp/_cookie_helpers.py | Implements fallback cookie parsing logic to handle malformed cookies while preserving subsequent valid cookies |
| tests/test_cookie_helpers.py | Adds comprehensive test coverage for fallback parser behavior and removes xfail marker from now-passing test |
| docs/spelling_wordlist.txt | Adds "unescaped" to the spelling dictionary to support the bugfix changelog entry |
| CHANGES/11632.bugfix.rst | Documents the bug fix for malformed cookie handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Backport to 3.13: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 82ce525 on top of patchback/backports/3.13/82ce525b3b3d11e1c9dbbe5ebc8aa0042a2cc7b8/pr-11724 Backporting merged PR #11724 into master
🤖 @patchback |
Backport to 3.14: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 82ce525 on top of patchback/backports/3.14/82ce525b3b3d11e1c9dbbe5ebc8aa0042a2cc7b8/pr-11724 Backporting merged PR #11724 into master
🤖 @patchback |
(cherry picked from commit 82ce525)
(cherry picked from commit 82ce525)
…fter a malformed cookie (#11730)
…fter a malformed cookie (#11729)
What do these changes do?
Ensure cookies are still parsed after a malformed cookie
Browsers and nodejs cookie parser are a lot more permissive so
we should handle similar cases.
Are there changes in behavior for the user?
we can handle more real world cookies
Is it a substantial burden for the maintainers to support this?
Its pretty similar to how node.js does it and they haven't substantively changed it in a while so probably not going to require much maint burden
Related issue number
fixes #11632