From a13b24835895632a2bc961816122b919ff0993c6 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 27 Jul 2021 12:41:13 -0400 Subject: [PATCH 1/7] add chardet dep to setup.py --- sdk/core/azure-core/setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/core/azure-core/setup.py b/sdk/core/azure-core/setup.py index b6c9f1b11d8d..635742dd8f83 100644 --- a/sdk/core/azure-core/setup.py +++ b/sdk/core/azure-core/setup.py @@ -68,6 +68,7 @@ install_requires=[ 'requests>=2.18.4', 'six>=1.11.0', + 'chardet>=3.0.2,<5', ], extras_require={ ":python_version<'3.0'": ['azure-nspkg'], From 2c9d57bfa35bacae7c27bbd4381ad748e0c8d5cb Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 27 Jul 2021 12:43:02 -0400 Subject: [PATCH 2/7] update changelog --- sdk/core/azure-core/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 30e247c25865..8daf86adb945 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -8,6 +8,8 @@ ### Key Bugs Fixed +- Added `chardet` dependency, because `requests` [removed this dependency](https://docs.python-requests.org/en/latest/community/updates/#id1), and our code relies on it. + ### Fixed - Not override "x-ms-client-request-id" if it already exists in the header. #17757 From 9de3b620fde695c9cad32f21e947492f0b6c9bc6 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 27 Jul 2021 14:49:54 -0400 Subject: [PATCH 3/7] remove chardet dependency, and improve text deserialization following httpx --- sdk/core/azure-core/CHANGELOG.md | 2 +- .../azure-core/azure/core/rest/_helpers.py | 34 +++++++++++-------- sdk/core/azure-core/azure/core/rest/_rest.py | 16 ++++++--- .../azure-core/azure/core/rest/_rest_py3.py | 18 ++++++---- sdk/core/azure-core/setup.py | 1 - .../test_rest_http_response_async.py | 6 ++-- .../test_rest_http_response.py | 6 ++-- 7 files changed, 49 insertions(+), 34 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 8daf86adb945..26fff4797fee 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -8,7 +8,7 @@ ### Key Bugs Fixed -- Added `chardet` dependency, because `requests` [removed this dependency](https://docs.python-requests.org/en/latest/community/updates/#id1), and our code relies on it. +- Removed `chardet` dependency in `azure.core.rest`. ### Fixed diff --git a/sdk/core/azure-core/azure/core/rest/_helpers.py b/sdk/core/azure-core/azure/core/rest/_helpers.py index 27e11299fdb5..ee463c9a6942 100644 --- a/sdk/core/azure-core/azure/core/rest/_helpers.py +++ b/sdk/core/azure-core/azure/core/rest/_helpers.py @@ -50,11 +50,6 @@ from urlparse import urlparse # type: ignore except ImportError: from urllib.parse import urlparse -try: - import cchardet as chardet -except ImportError: # pragma: no cover - import chardet # type: ignore -from ..exceptions import ResponseNotReadError ################################### TYPES SECTION ######################### @@ -285,22 +280,31 @@ def from_pipeline_transport_request_helper(request_class, pipeline_transport_req ) def get_charset_encoding(response): + # type: (...) -> Optional[str] content_type = response.headers.get("Content-Type") if not content_type: return None _, params = cgi.parse_header(content_type) encoding = params.get('charset') # -> utf-8 - if encoding is None: - if content_type in ("application/json", "application/rdap+json"): - # RFC 7159 states that the default encoding is UTF-8. - # RFC 7483 defines application/rdap+json - encoding = "utf-8" - else: - try: - encoding = chardet.detect(response.content)["encoding"] - except ResponseNotReadError: - pass if encoding is None or not lookup_encoding(encoding): return None return encoding + +def decode_to_text(encoding, content): + # type: (Optional[str], bytes) -> str + if encoding == "utf-8": + encoding = "utf-8-sig" + if encoding: + return content.decode(encoding) + # 1. We try utf-8-sig + decoder = codecs.getincrementaldecoder("utf-8-sig")(errors="strict") + try: + decoder.decode(content) + except UnicodeDecodeError: + # Could not decode as UTF-8. Use Windows 1252. + decoder = codecs.getincrementaldecoder("cp1252")(errors="replace") + else: + # Can decode as UTF-8. Use UTF-8 with lenient error settings. + decoder = codecs.getincrementaldecoder("utf-8")(errors="replace") + return decoder.decode(content) diff --git a/sdk/core/azure-core/azure/core/rest/_rest.py b/sdk/core/azure-core/azure/core/rest/_rest.py index 56823ab6d927..5f62ba3b62f7 100644 --- a/sdk/core/azure-core/azure/core/rest/_rest.py +++ b/sdk/core/azure-core/azure/core/rest/_rest.py @@ -42,6 +42,7 @@ to_pipeline_transport_request_helper, from_pipeline_transport_request_helper, get_charset_encoding, + decode_to_text, ) from ..exceptions import ResponseNotReadError if TYPE_CHECKING: @@ -205,6 +206,7 @@ def __init__(self, **kwargs): self._json = None # this is filled in ContentDecodePolicy, when we deserialize self._connection_data_block_size = None # type: Optional[int] self._content = None # type: Optional[bytes] + self._text = None # type: Optional[str] @property def url(self): @@ -221,7 +223,8 @@ def encoding(self): try: return self._encoding except AttributeError: - return get_charset_encoding(self) + self._encoding = get_charset_encoding(self) # type: Optional[str] + return self._encoding @encoding.setter def encoding(self, value): @@ -233,10 +236,13 @@ def encoding(self, value): def text(self): # type: (...) -> str """Returns the response body as a string""" - encoding = self.encoding - if encoding == "utf-8" or encoding is None: - encoding = "utf-8-sig" - return self.content.decode(encoding) + if self._text is None: + content = self.content + if not content: + self._text = "" + else: + self._text = decode_to_text(self.encoding, self.content) + return self._text def json(self): # type: (...) -> Any diff --git a/sdk/core/azure-core/azure/core/rest/_rest_py3.py b/sdk/core/azure-core/azure/core/rest/_rest_py3.py index 80a91004c38f..6f6d5672dd84 100644 --- a/sdk/core/azure-core/azure/core/rest/_rest_py3.py +++ b/sdk/core/azure-core/azure/core/rest/_rest_py3.py @@ -55,7 +55,8 @@ format_parameters, to_pipeline_transport_request_helper, from_pipeline_transport_request_helper, - get_charset_encoding + get_charset_encoding, + decode_to_text, ) from ._helpers_py3 import set_content_body from ..exceptions import ResponseNotReadError @@ -235,6 +236,7 @@ def __init__( self._connection_data_block_size = None self._json = None # this is filled in ContentDecodePolicy, when we deserialize self._content = None # type: Optional[bytes] + self._text = None # type: Optional[str] @property def url(self) -> str: @@ -249,7 +251,8 @@ def encoding(self) -> Optional[str]: try: return self._encoding except AttributeError: - return get_charset_encoding(self) + self._encoding: Optional[str] = get_charset_encoding(self) + return self._encoding @encoding.setter def encoding(self, value: str) -> None: @@ -259,10 +262,13 @@ def encoding(self, value: str) -> None: @property def text(self) -> str: """Returns the response body as a string""" - encoding = self.encoding - if encoding == "utf-8" or encoding is None: - encoding = "utf-8-sig" - return self.content.decode(encoding) + if self._text is None: + content = self.content + if not content: + self._text = "" + else: + self._text = decode_to_text(self.encoding, self.content) + return self._text def json(self) -> Any: """Returns the whole body as a json object. diff --git a/sdk/core/azure-core/setup.py b/sdk/core/azure-core/setup.py index 635742dd8f83..b6c9f1b11d8d 100644 --- a/sdk/core/azure-core/setup.py +++ b/sdk/core/azure-core/setup.py @@ -68,7 +68,6 @@ install_requires=[ 'requests>=2.18.4', 'six>=1.11.0', - 'chardet>=3.0.2,<5', ], extras_require={ ":python_version<'3.0'": ['azure-nspkg'], diff --git a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py index 317b74c3bac0..61066ee4a35e 100644 --- a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py +++ b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py @@ -149,7 +149,7 @@ async def test_response_no_charset_with_ascii_content(send_request): assert response.headers["Content-Type"] == "text/plain" assert response.status_code == 200 - assert response.encoding == 'ascii' + assert response.encoding is None content = await response.read() assert content == b"Hello, world!" assert response.text == "Hello, world!" @@ -166,7 +166,7 @@ async def test_response_no_charset_with_iso_8859_1_content(send_request): ) await response.read() assert response.text == u"Accented: Österreich" - assert response.encoding == 'ISO-8859-1' + assert response.encoding is None # NOTE: aiohttp isn't liking this # @pytest.mark.asyncio @@ -187,7 +187,7 @@ async def test_json(send_request): ) await response.read() assert response.json() == {"greeting": "hello", "recipient": "world"} - assert response.encoding == 'utf-8' + assert response.encoding is None @pytest.mark.asyncio async def test_json_with_specified_encoding(send_request): diff --git a/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py b/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py index 83255119f4ab..804a98b8890e 100644 --- a/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py +++ b/sdk/core/azure-core/tests/testserver_tests/test_rest_http_response.py @@ -138,7 +138,7 @@ def test_response_no_charset_with_ascii_content(send_request): assert response.headers["Content-Type"] == "text/plain" assert response.status_code == 200 - assert response.encoding == 'ascii' + assert response.encoding is None assert response.text == "Hello, world!" @@ -151,7 +151,7 @@ def test_response_no_charset_with_iso_8859_1_content(send_request): request=HttpRequest("GET", "/encoding/iso-8859-1"), ) assert response.text == u"Accented: Österreich" - assert response.encoding == 'ISO-8859-1' + assert response.encoding is None def test_response_set_explicit_encoding(send_request): # Deliberately incorrect charset @@ -168,7 +168,7 @@ def test_json(send_request): request=HttpRequest("GET", "/basic/json"), ) assert response.json() == {"greeting": "hello", "recipient": "world"} - assert response.encoding == 'utf-8-sig' # for requests, we use utf-8-sig instead of utf-8 bc of requests behavior + assert response.encoding is None def test_json_with_specified_encoding(send_request): response = send_request( From 2be52d4e13338444c54e3501519deb230ab7c0db Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Tue, 27 Jul 2021 16:17:42 -0400 Subject: [PATCH 4/7] remove cp1252 decoding --- sdk/core/azure-core/azure/core/rest/_helpers.py | 12 +----------- .../async_tests/test_rest_http_response_async.py | 2 +- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/sdk/core/azure-core/azure/core/rest/_helpers.py b/sdk/core/azure-core/azure/core/rest/_helpers.py index ee463c9a6942..4975d09d655c 100644 --- a/sdk/core/azure-core/azure/core/rest/_helpers.py +++ b/sdk/core/azure-core/azure/core/rest/_helpers.py @@ -297,14 +297,4 @@ def decode_to_text(encoding, content): encoding = "utf-8-sig" if encoding: return content.decode(encoding) - # 1. We try utf-8-sig - decoder = codecs.getincrementaldecoder("utf-8-sig")(errors="strict") - try: - decoder.decode(content) - except UnicodeDecodeError: - # Could not decode as UTF-8. Use Windows 1252. - decoder = codecs.getincrementaldecoder("cp1252")(errors="replace") - else: - # Can decode as UTF-8. Use UTF-8 with lenient error settings. - decoder = codecs.getincrementaldecoder("utf-8")(errors="replace") - return decoder.decode(content) + return codecs.getincrementaldecoder("utf-8-sig")(errors="replace").decode(content) diff --git a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py index 61066ee4a35e..47286b76254b 100644 --- a/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py +++ b/sdk/core/azure-core/tests/testserver_tests/async_tests/test_rest_http_response_async.py @@ -165,7 +165,7 @@ async def test_response_no_charset_with_iso_8859_1_content(send_request): request=HttpRequest("GET", "/encoding/iso-8859-1"), ) await response.read() - assert response.text == u"Accented: Österreich" + assert response.text == "Accented: �sterreich" # aiohttp is having diff behavior than requests assert response.encoding is None # NOTE: aiohttp isn't liking this From 916aafca22ff72d19c834b35f5aa376c5c421281 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 2 Aug 2021 11:27:32 -0400 Subject: [PATCH 5/7] update encoding docstring --- sdk/core/azure-core/azure/core/rest/_rest.py | 8 ++++++-- sdk/core/azure-core/azure/core/rest/_rest_py3.py | 8 ++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/sdk/core/azure-core/azure/core/rest/_rest.py b/sdk/core/azure-core/azure/core/rest/_rest.py index 5f62ba3b62f7..295051a83c50 100644 --- a/sdk/core/azure-core/azure/core/rest/_rest.py +++ b/sdk/core/azure-core/azure/core/rest/_rest.py @@ -217,8 +217,12 @@ def url(self): @property def encoding(self): # type: (...) -> Optional[str] - """Returns the response encoding. By default, is specified - by the response Content-Type header. + """Returns the response encoding. + + :return: The response encoding. We either return the encoding set by the user, + or try extracting the encoding from the response's content type. If all fails, + we return `None`. + :rtype: optional[str] """ try: return self._encoding diff --git a/sdk/core/azure-core/azure/core/rest/_rest_py3.py b/sdk/core/azure-core/azure/core/rest/_rest_py3.py index 6f6d5672dd84..af957f767a39 100644 --- a/sdk/core/azure-core/azure/core/rest/_rest_py3.py +++ b/sdk/core/azure-core/azure/core/rest/_rest_py3.py @@ -245,8 +245,12 @@ def url(self) -> str: @property def encoding(self) -> Optional[str]: - """Returns the response encoding. By default, is specified - by the response Content-Type header. + """Returns the response encoding. + + :return: The response encoding. We either return the encoding set by the user, + or try extracting the encoding from the response's content type. If all fails, + we return `None`. + :rtype: optional[str] """ try: return self._encoding From a7a1a85048b9548c24ec245619455d12bf5cb30a Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 2 Aug 2021 15:15:20 -0400 Subject: [PATCH 6/7] change changelog to breaking change in provisional --- sdk/core/azure-core/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 26fff4797fee..e15a5ecbf594 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -4,11 +4,11 @@ ### Features Added -### Breaking Changes +### Breaking Changes in the Provisional `azure.core.rest` package -### Key Bugs Fixed +- Removed `chardet` dependency in `azure.core.rest`. Breaking because we removed the step that used `chardet` to inspect the response encoding -- Removed `chardet` dependency in `azure.core.rest`. +### Key Bugs Fixed ### Fixed From be24aa71c632a8bc6993f9d72494a8c6e0b07f03 Mon Sep 17 00:00:00 2001 From: iscai-msft <43154838+iscai-msft@users.noreply.github.com> Date: Tue, 3 Aug 2021 11:09:25 -0400 Subject: [PATCH 7/7] Update CHANGELOG.md --- sdk/core/azure-core/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index c30e5bf16997..9a8bf9291e74 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -8,7 +8,7 @@ ### Breaking Changes in the Provisional `azure.core.rest` package -- Removed `chardet` dependency in `azure.core.rest`. Breaking because we removed the step that used `chardet` to inspect the response encoding +- `azure.core.rest` will not try to guess the `charset` anymore if it was impossible to extract it from `HttpResponse` analysis. This removes our dependency on `charset`. ### Key Bugs Fixed