Skip to content
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

Fixed JSON response character set detection. #1339

Merged
merged 1 commit into from
Oct 27, 2016

Conversation

kimvais
Copy link
Contributor

@kimvais kimvais commented Oct 27, 2016

What do these changes do?

This disables running chardet on responses with CONTENT_TYPE: application/json as this is a HUGE performance hit.

RFC 7519 states that JSON MUST be encoded in UTF and that
the default encoding, in absence of charset in CONTENT_TYPE header is UTF-8.

This vastly improves performance when doing many small requests in environments
where the server does not specify the character by omitting chardet.

Are there changes in behavior for the user?

This does not change the behaviour unless a non-conforming server sends a HTTP response using an invalid encoding without specifying the encoding correctly in the header.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep alphabetical order, the file is sorted by names.
  • Add a new entry to CHANGES.rst
    • Choose any open position to avoid merge conflicts with other PRs.
    • Add a link to the issue you are fixing (if any) using #issue_number format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.

RFC 7519 states that JSON *MUST* be encoded in UTF and that
the default encoding, in absence of charset in CONTENT_TYPE header is UTF-8.

This vastly improves performce when doing many small requests in environments
where the server does not specify the character by omitting chardet.
@codecov-io
Copy link

codecov-io commented Oct 27, 2016

Current coverage is 98.74% (diff: 100%)

Merging #1339 into master will decrease coverage by 0.03%

@@             master      #1339   diff @@
==========================================
  Files            29         29          
  Lines          6523       6525     +2   
  Methods           0          0          
  Messages          0          0          
  Branches       1093       1094     +1   
==========================================
  Hits           6443       6443          
- Misses           36         37     +1   
- Partials         44         45     +1   

Powered by Codecov. Last update 7cf683c...80831fc

@asvetlov asvetlov merged commit 416a445 into aio-libs:master Oct 27, 2016
@asvetlov
Copy link
Member

Thanks

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants