From e8076b5f2e78adacc63c76208685fb1e65d66ea7 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 15 Jun 2021 13:19:03 +0100 Subject: [PATCH 01/16] Treat warnings as errors --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index c860d819c0..16d5fd6ae3 100644 --- a/setup.cfg +++ b/setup.cfg @@ -15,7 +15,7 @@ profile = black combine_as_imports = True [tool:pytest] -addopts = -rxXs +addopts = -rxXs -Werror markers = copied_from(source, changes=None): mark test as copied from somewhere else, along with a description of changes made to accodomate e.g. our test setup From 3881ee28f30568bb7ba50b4537a41f9d70a8b554 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 15 Jun 2021 13:35:33 +0100 Subject: [PATCH 02/16] Defensive programming in Client.__del__ to avoid possible warnings on partially initialized instances --- httpx/_client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index 11c8e12351..e4434588a2 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -1244,7 +1244,10 @@ def __exit__( transport.__exit__(exc_type, exc_value, traceback) def __del__(self) -> None: - if self._state == ClientState.OPENED: + # We use 'getattr' here, to manage the case where '__del__()' is called + # on a partically initiallized instance that raised an exception during + # the call to '__init__()'. + if getattr(self, '_state') == ClientState.OPENED: self.close() @@ -1942,7 +1945,10 @@ async def __aexit__( await proxy.__aexit__(exc_type, exc_value, traceback) def __del__(self) -> None: - if self._state == ClientState.OPENED: + # We use 'getattr' here, to manage the case where '__del__()' is called + # on a partically initiallized instance that raised an exception during + # the call to '__init__()'. + if getattr(self, '_state') == ClientState.OPENED: # Unlike the sync case, we cannot silently close the client when # it is garbage collected, because `.aclose()` is an async operation, # but `__del__` is not. From 94721edb959acb6f074a5ee385b9e144c5d8ea61 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 15 Jun 2021 13:37:01 +0100 Subject: [PATCH 03/16] Linting --- httpx/_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index e4434588a2..7da7e38ffd 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -1247,7 +1247,7 @@ def __del__(self) -> None: # We use 'getattr' here, to manage the case where '__del__()' is called # on a partically initiallized instance that raised an exception during # the call to '__init__()'. - if getattr(self, '_state') == ClientState.OPENED: + if getattr(self, "_state") == ClientState.OPENED: self.close() @@ -1948,7 +1948,7 @@ def __del__(self) -> None: # We use 'getattr' here, to manage the case where '__del__()' is called # on a partically initiallized instance that raised an exception during # the call to '__init__()'. - if getattr(self, '_state') == ClientState.OPENED: + if getattr(self, "_state") == ClientState.OPENED: # Unlike the sync case, we cannot silently close the client when # it is garbage collected, because `.aclose()` is an async operation, # but `__del__` is not. From 6e4784fc6c476a0026c07c61415933532068bfea Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 15 Jun 2021 13:41:01 +0100 Subject: [PATCH 04/16] Ignore linting getattr errors in __del__ --- httpx/_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index 7da7e38ffd..eb0a57dead 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -1247,7 +1247,7 @@ def __del__(self) -> None: # We use 'getattr' here, to manage the case where '__del__()' is called # on a partically initiallized instance that raised an exception during # the call to '__init__()'. - if getattr(self, "_state") == ClientState.OPENED: + if getattr(self, "_state") == ClientState.OPENED: # noqa: B009 self.close() @@ -1948,7 +1948,7 @@ def __del__(self) -> None: # We use 'getattr' here, to manage the case where '__del__()' is called # on a partically initiallized instance that raised an exception during # the call to '__init__()'. - if getattr(self, "_state") == ClientState.OPENED: + if getattr(self, "_state") == ClientState.OPENED: # noqa: B009 # Unlike the sync case, we cannot silently close the client when # it is garbage collected, because `.aclose()` is an async operation, # but `__del__` is not. From 05389e8900936aa5a3c148ca395377b5fc32d043 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 15 Jun 2021 13:43:32 +0100 Subject: [PATCH 05/16] getattr requires a default --- httpx/_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index eb0a57dead..5f5f6619cb 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -1247,7 +1247,7 @@ def __del__(self) -> None: # We use 'getattr' here, to manage the case where '__del__()' is called # on a partically initiallized instance that raised an exception during # the call to '__init__()'. - if getattr(self, "_state") == ClientState.OPENED: # noqa: B009 + if getattr(self, "_state", None) == ClientState.OPENED: # noqa: B009 self.close() @@ -1948,7 +1948,7 @@ def __del__(self) -> None: # We use 'getattr' here, to manage the case where '__del__()' is called # on a partically initiallized instance that raised an exception during # the call to '__init__()'. - if getattr(self, "_state") == ClientState.OPENED: # noqa: B009 + if getattr(self, "_state", None) == ClientState.OPENED: # noqa: B009 # Unlike the sync case, we cannot silently close the client when # it is garbage collected, because `.aclose()` is an async operation, # but `__del__` is not. From 62f2d9b3e86c3a00cc1a68f69acb1db429849a30 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 15 Jun 2021 14:19:55 +0100 Subject: [PATCH 06/16] Tighten up closing of auth_flow generators --- httpx/_client.py | 90 ++++++++++++++++++++++++++---------------------- 1 file changed, 48 insertions(+), 42 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index 5f5f6619cb..c6e1efbe6b 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -897,32 +897,35 @@ def _send_handling_auth( history: typing.List[Response], ) -> Response: auth_flow = auth.sync_auth_flow(request) - request = next(auth_flow) + try: + request = next(auth_flow) - for hook in self._event_hooks["request"]: - hook(request) + for hook in self._event_hooks["request"]: + hook(request) - while True: - response = self._send_handling_redirects( - request, - timeout=timeout, - allow_redirects=allow_redirects, - history=history, - ) - try: + while True: + response = self._send_handling_redirects( + request, + timeout=timeout, + allow_redirects=allow_redirects, + history=history, + ) try: - next_request = auth_flow.send(response) - except StopIteration: - return response + try: + next_request = auth_flow.send(response) + except StopIteration: + return response - response.history = list(history) - response.read() - request = next_request - history.append(response) + response.history = list(history) + response.read() + request = next_request + history.append(response) - except Exception as exc: - response.close() - raise exc + except Exception as exc: + response.close() + raise exc + finally: + auth_flow.close() def _send_handling_redirects( self, @@ -1591,32 +1594,35 @@ async def _send_handling_auth( history: typing.List[Response], ) -> Response: auth_flow = auth.async_auth_flow(request) - request = await auth_flow.__anext__() + try: + request = await auth_flow.__anext__() - for hook in self._event_hooks["request"]: - await hook(request) + for hook in self._event_hooks["request"]: + await hook(request) - while True: - response = await self._send_handling_redirects( - request, - timeout=timeout, - allow_redirects=allow_redirects, - history=history, - ) - try: + while True: + response = await self._send_handling_redirects( + request, + timeout=timeout, + allow_redirects=allow_redirects, + history=history, + ) try: - next_request = await auth_flow.asend(response) - except StopAsyncIteration: - return response + try: + next_request = await auth_flow.asend(response) + except StopAsyncIteration: + return response - response.history = list(history) - await response.aread() - request = next_request - history.append(response) + response.history = list(history) + await response.aread() + request = next_request + history.append(response) - except Exception as exc: - await response.aclose() - raise exc + except Exception as exc: + await response.aclose() + raise exc + finally: + await auth_flow.aclose() async def _send_handling_redirects( self, From eb6bd86ce78e827a53be4f1d3c27579dd0481655 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Tue, 15 Jun 2021 14:27:31 +0100 Subject: [PATCH 07/16] Switch multipart test to open file in a context manager --- tests/test_multipart.py | 57 +++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/tests/test_multipart.py b/tests/test_multipart.py index 9eb62f785b..3824fb6bd6 100644 --- a/tests/test_multipart.py +++ b/tests/test_multipart.py @@ -107,34 +107,35 @@ def test_multipart_encode(tmp_path: typing.Any) -> None: "e": True, "f": "", } - files = {"file": ("name.txt", open(path, "rb"))} - - with mock.patch("os.urandom", return_value=os.urandom(16)): - boundary = os.urandom(16).hex() - - headers, stream = encode_request(data=data, files=files) - assert isinstance(stream, typing.Iterable) - - content = ( - '--{0}\r\nContent-Disposition: form-data; name="a"\r\n\r\n1\r\n' - '--{0}\r\nContent-Disposition: form-data; name="b"\r\n\r\nC\r\n' - '--{0}\r\nContent-Disposition: form-data; name="c"\r\n\r\n11\r\n' - '--{0}\r\nContent-Disposition: form-data; name="c"\r\n\r\n22\r\n' - '--{0}\r\nContent-Disposition: form-data; name="c"\r\n\r\n33\r\n' - '--{0}\r\nContent-Disposition: form-data; name="d"\r\n\r\n\r\n' - '--{0}\r\nContent-Disposition: form-data; name="e"\r\n\r\ntrue\r\n' - '--{0}\r\nContent-Disposition: form-data; name="f"\r\n\r\n\r\n' - '--{0}\r\nContent-Disposition: form-data; name="file";' - ' filename="name.txt"\r\n' - "Content-Type: text/plain\r\n\r\n\r\n" - "--{0}--\r\n" - "".format(boundary).encode("ascii") - ) - assert headers == { - "Content-Type": f"multipart/form-data; boundary={boundary}", - "Content-Length": str(len(content)), - } - assert content == b"".join(stream) + with open(path, "rb") as input_file: + files = {"file": ("name.txt", input_file)} + + with mock.patch("os.urandom", return_value=os.urandom(16)): + boundary = os.urandom(16).hex() + + headers, stream = encode_request(data=data, files=files) + assert isinstance(stream, typing.Iterable) + + content = ( + '--{0}\r\nContent-Disposition: form-data; name="a"\r\n\r\n1\r\n' + '--{0}\r\nContent-Disposition: form-data; name="b"\r\n\r\nC\r\n' + '--{0}\r\nContent-Disposition: form-data; name="c"\r\n\r\n11\r\n' + '--{0}\r\nContent-Disposition: form-data; name="c"\r\n\r\n22\r\n' + '--{0}\r\nContent-Disposition: form-data; name="c"\r\n\r\n33\r\n' + '--{0}\r\nContent-Disposition: form-data; name="d"\r\n\r\n\r\n' + '--{0}\r\nContent-Disposition: form-data; name="e"\r\n\r\ntrue\r\n' + '--{0}\r\nContent-Disposition: form-data; name="f"\r\n\r\n\r\n' + '--{0}\r\nContent-Disposition: form-data; name="file";' + ' filename="name.txt"\r\n' + "Content-Type: text/plain\r\n\r\n\r\n" + "--{0}--\r\n" + "".format(boundary).encode("ascii") + ) + assert headers == { + "Content-Type": f"multipart/form-data; boundary={boundary}", + "Content-Length": str(len(content)), + } + assert content == b"".join(stream) def test_multipart_encode_unicode_file_contents() -> None: From ecc1dd906a3a88c23c72af65f445a73e63f66d82 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 12:39:43 +0100 Subject: [PATCH 08/16] Ignore warnings on uvicorn --- setup.cfg | 3 +++ 1 file changed, 3 insertions(+) diff --git a/setup.cfg b/setup.cfg index 16d5fd6ae3..555f73d9bb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -16,6 +16,9 @@ combine_as_imports = True [tool:pytest] addopts = -rxXs -Werror +filterwarnings = + error + default:::uvicorn markers = copied_from(source, changes=None): mark test as copied from somewhere else, along with a description of changes made to accodomate e.g. our test setup From 25392a2773d7ddb241e2d767fe4a00f7e45bf039 Mon Sep 17 00:00:00 2001 From: jianghang Date: Wed, 16 Jun 2021 19:56:53 +0800 Subject: [PATCH 09/16] Drop -Werror from addopts --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 555f73d9bb..fc8a1e64e9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -15,7 +15,7 @@ profile = black combine_as_imports = True [tool:pytest] -addopts = -rxXs -Werror +addopts = -rxXs filterwarnings = error default:::uvicorn From f4aebcedff4b28581785cd49008cdad2d006ea6e Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 13:05:05 +0100 Subject: [PATCH 10/16] Warings specified entirely in 'filterwarnings' section --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 555f73d9bb..fc8a1e64e9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -15,7 +15,7 @@ profile = black combine_as_imports = True [tool:pytest] -addopts = -rxXs -Werror +addopts = -rxXs filterwarnings = error default:::uvicorn From dcbf4d755e54eb6b13747db673f894e8163a5c54 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 13:10:53 +0100 Subject: [PATCH 11/16] Use ssl.PROTOCOL_TLS_CLIENT instead of deprecated ssl.PROTOCOL_TLS --- httpx/_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpx/_config.py b/httpx/_config.py index 837519afb5..e20290a272 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -153,7 +153,7 @@ def _create_default_ssl_context(self) -> ssl.SSLContext: Creates the default SSLContext object that's used for both verified and unverified connections. """ - context = ssl.SSLContext(ssl.PROTOCOL_TLS) + context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) context.options |= ssl.OP_NO_SSLv2 context.options |= ssl.OP_NO_SSLv3 context.options |= ssl.OP_NO_TLSv1 From ed9aabfeff6c18652db918bd0628c94d2513487a Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 14:09:19 +0100 Subject: [PATCH 12/16] Push 'check_hostname = False' above 'context.verify_mode = ssl.CERT_NONE' --- httpx/_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpx/_config.py b/httpx/_config.py index e20290a272..65b80606bb 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -90,8 +90,8 @@ def load_ssl_context_no_verify(self) -> ssl.SSLContext: Return an SSL context for unverified connections. """ context = self._create_default_ssl_context() - context.verify_mode = ssl.CERT_NONE context.check_hostname = False + context.verify_mode = ssl.CERT_NONE self._load_client_certs(context) return context From c433d74496239e8c81929430eb8df3e9df73c72d Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 14:27:07 +0100 Subject: [PATCH 13/16] Introduce set_minimum_tls_version_1_2 compatible across different python versions --- httpx/_compat.py | 13 +++++++++++++ httpx/_config.py | 6 ++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/httpx/_compat.py b/httpx/_compat.py index 47c12ba199..6f11bd151e 100644 --- a/httpx/_compat.py +++ b/httpx/_compat.py @@ -1,6 +1,19 @@ +import ssl +import sys + # `contextlib.asynccontextmanager` exists from Python 3.7 onwards. # For 3.6 we require the `async_generator` package for a backported version. try: from contextlib import asynccontextmanager # type: ignore except ImportError: # pragma: no cover from async_generator import asynccontextmanager # type: ignore # noqa + + +def set_minimum_tls_version_1_2(context: ssl.SSLContext): + if sys.version_info >= (3, 10): + context.minimum_version = ssl.TLSVersion.TLSv1_2 + else: + context.options |= ssl.OP_NO_SSLv2 + context.options |= ssl.OP_NO_SSLv3 + context.options |= ssl.OP_NO_TLSv1 + context.options |= ssl.OP_NO_TLSv1_1 diff --git a/httpx/_config.py b/httpx/_config.py index 65b80606bb..9d29f9f2f1 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -6,6 +6,7 @@ import certifi +from ._compat import set_minimum_tls_version_1_2 from ._models import URL, Headers from ._types import CertTypes, HeaderTypes, TimeoutTypes, URLTypes, VerifyTypes from ._utils import get_ca_bundle_from_env, get_logger @@ -154,10 +155,7 @@ def _create_default_ssl_context(self) -> ssl.SSLContext: and unverified connections. """ context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - context.options |= ssl.OP_NO_SSLv2 - context.options |= ssl.OP_NO_SSLv3 - context.options |= ssl.OP_NO_TLSv1 - context.options |= ssl.OP_NO_TLSv1_1 + set_minimum_tls_version_1_2(context) context.options |= ssl.OP_NO_COMPRESSION context.set_ciphers(DEFAULT_CIPHERS) From 76992dc827e7e50add8a15ce906a0acca3d14bae Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 14:29:20 +0100 Subject: [PATCH 14/16] Commenting --- httpx/_compat.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/httpx/_compat.py b/httpx/_compat.py index 6f11bd151e..2eb936ea81 100644 --- a/httpx/_compat.py +++ b/httpx/_compat.py @@ -13,6 +13,8 @@ def set_minimum_tls_version_1_2(context: ssl.SSLContext): if sys.version_info >= (3, 10): context.minimum_version = ssl.TLSVersion.TLSv1_2 else: + # These become deprecated in favor of 'context.minimum_version' + # from Python 3.10 onwards. context.options |= ssl.OP_NO_SSLv2 context.options |= ssl.OP_NO_SSLv3 context.options |= ssl.OP_NO_TLSv1 From 7561f7f47ed67a1a61e26d1864d020838e42e6ad Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 14:30:18 +0100 Subject: [PATCH 15/16] Add missing annotation --- httpx/_compat.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpx/_compat.py b/httpx/_compat.py index 2eb936ea81..ecb78ccf4b 100644 --- a/httpx/_compat.py +++ b/httpx/_compat.py @@ -9,7 +9,7 @@ from async_generator import asynccontextmanager # type: ignore # noqa -def set_minimum_tls_version_1_2(context: ssl.SSLContext): +def set_minimum_tls_version_1_2(context: ssl.SSLContext) -> None: if sys.version_info >= (3, 10): context.minimum_version = ssl.TLSVersion.TLSv1_2 else: From e6594a2e1e953c535b201f33a14acd49b666c9c5 Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Wed, 16 Jun 2021 14:37:21 +0100 Subject: [PATCH 16/16] Exclude _compat from coverage --- httpx/_compat.py | 6 +++++- setup.cfg | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/httpx/_compat.py b/httpx/_compat.py index ecb78ccf4b..98a3e37b82 100644 --- a/httpx/_compat.py +++ b/httpx/_compat.py @@ -1,3 +1,7 @@ +""" +The _compat module is used for code which requires branching between different +Python environments. It is excluded from the code coverage checks. +""" import ssl import sys @@ -5,7 +9,7 @@ # For 3.6 we require the `async_generator` package for a backported version. try: from contextlib import asynccontextmanager # type: ignore -except ImportError: # pragma: no cover +except ImportError: from async_generator import asynccontextmanager # type: ignore # noqa diff --git a/setup.cfg b/setup.cfg index fc8a1e64e9..2a3bc303f7 100644 --- a/setup.cfg +++ b/setup.cfg @@ -23,5 +23,5 @@ markers = copied_from(source, changes=None): mark test as copied from somewhere else, along with a description of changes made to accodomate e.g. our test setup [coverage:run] -omit = venv/* +omit = venv/*, httpx/_compat.py include = httpx/*, tests/*