-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix badly encoded charset crashing instead of falling back to detector #9160
Conversation
93c7223 to
38e20f8
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9160 +/- ##
=======================================
Coverage 98.31% 98.31%
=======================================
Files 107 107
Lines 34461 34475 +14
Branches 4089 4090 +1
=======================================
+ Hits 33879 33893 +14
Misses 411 411
Partials 171 171
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Weird that the documentation doesn't mention these exceptions, but I think the change is correct. |
Backport to 3.10: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 8ad118b on top of patchback/backports/3.10/8ad118b1d8b83ce0c0b61ae0cf8019c9a2fcd5ae/pr-9160 Backporting merged PR #9160 into master
🤖 @patchback |
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 8ad118b on top of patchback/backports/3.11/8ad118b1d8b83ce0c0b61ae0cf8019c9a2fcd5ae/pr-9160 Backporting merged PR #9160 into master
🤖 @patchback |
|
Could you follow the above instructions for backports please? |
…or (aio-libs#9160) (cherry picked from commit 8ad118b) # Conflicts: # tests/test_client_response.py
…or (aio-libs#9160) (cherry picked from commit 8ad118b) # Conflicts: # tests/test_client_response.py
What do these changes do?
They fix a crash that occurs when processing a
Content-Typeheader that is itself badly encoded.Example I saw in the wild was:
where I got:
Yet to me, this falls under the "not understood by Python" case of the documentation:
Are there changes in behavior for the user?
Such websites no longer cause a
UnicodeEncodeErrorthat crashesClientResponse.get_encoding()(and ultimatelyClientResponse.text()) and will instead fallback as if it was aLookupErroron the encoding. This is the behaviour I expected to see, hence the PR.Is it a substantial burden for the maintainers to support this?
I don't think so, it might even lead to issues not being raised when they would have.
Related issue number
Sort of related to #207 and #3279.
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.