psf / psf/requests

[Suggestion] Simplify charset handling

Open
#1,737 6 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Breaking API Change Needs BDFL Input
Dominant language
Python
Stars
54.3k
Forks
10.4k
Avg merge
16h 43m
Merged PRs (30d)
3

Description

To cut to the chase, here are my suggestions:

  • Remove automatic character set detection (charade) from the library
  • Have response.encoding represent the charset from the Content-Type header
  • Mention the caveat in the documentation
Long version

There seems to be a lot of confusion regarding how the .text property works. After getting into some trouble with it myself, I searched the issues list, and found a dozen or so issues, all boiling down to the same mismatch between users' expectations and the intent of the library designers.
#147 - bytecode string returned when page has charset=UTF-8
#156 - get_unicode_from_response does not check charsets from meta tags
#592 - Internal encoding detection doesn't match external chardet call
#654 - requests.get() ignores charset=UTF-8 and BOM
#765 - Chardet sometimes fails and force the wrong encoding
#861 - parsing encoding utf-8 page doesn't as expected
#1087 - Encodings from content
#1150 - On some pages requests detect encoding incorrectly
#1546 - use a default encoding in Response's text property
#1589 - Make sure content is checked when setting encoding
#1604 - Response.text returns improperly decoded text
#1683 - models.text Behaviour (encoding choice)

(It must be tiring to have the same conversation over and over again. I hope I'm being helpful here and not just piling on).

The argument seems to be:

  • As an HTTP library, requests should not know or care about HTML and META attributes
  • RFC 2616 states that if no charset is defined, "text/*" media types should be regarded as ISO-8859-1

I accept both these arguments. However, the documentation seems a bit coy, saying that "Requests makes an educated guess about the encoding", implying chardet/charade. In practice, for any content with a "text" media subtype, charade will not be used unless the user explicitly sets the response.encoding to None before reading the .text property.

Additionally, while ISO-8859-1 can be used as a default, won't it make more sense to handle that default in .text and not in the get_encoding_from_headers method? This way, the encoding property will be None if indeed no encoding is specified, allowing the user to make the decision on how to proceed.

If you're going to keep the .text property, I think it should do a simple decoding if the charset is specified in the headers, and throw an exception otherwise. This way is much less confusing than the state of affairs now. Additionally, the documentation should contain a warning not to use it for arbitrary web pages, and perhaps a code sample showing the proper way to do it.


import re
import charade
import requests

def get_encodings_from_content(content):
    charset_re = re.compile(r'<meta.*?charset=["\']*(.+?)["\'>]', flags=re.I)
    pragma_re = re.compile(r'<meta.*?content=["\']*;?charset=(.+?)["\'>]', flags=re.I)
    xml_re = re.compile(r'^<\?xml.*?encoding=["\']*(.+?)["\'>]')

    # FIXME: Does not work in python 3
    return (charset_re.findall(content) +
            pragma_re.findall(content) +
            xml_re.findall(content))

r = requests.get('https://example.com/page.html')
if "charset" not in r.headers.get("content-type", ""):
    encodings = get_encodings_from_content(r.content)
    if encodings:
        r.encoding = encodings[0]
    else:
        r.encoding = charade.detect(r.content)['encoding']

html = r.text

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Response.encoding and Response.text entry points and the get_encoding_from_headers method mentioned in the issue, then review the charset-related documentation. Compare the current behavior with the proposed header-based handling and clarify how missing charsets should be represented; done means the behavior and its caveat are documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, documentation
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.