diff --git a/CHANGES/4431.bugfix b/CHANGES/4431.bugfix new file mode 100644 index 0000000000..bb325354c5 --- /dev/null +++ b/CHANGES/4431.bugfix @@ -0,0 +1 @@ +Fixed HTTP client requests to honor ``no_proxy`` environment variables. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 20f110dea9..7770808d2a 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -265,6 +265,7 @@ Stanislav Prokop Stefan Tjarks Stepan Pletnev Stephan Jaensch +Stephen Cirelli Stephen Granade Steven Seguin Sunghyun Hwang diff --git a/aiohttp/client.py b/aiohttp/client.py index ec71b67a45..62b18d07ff 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -9,6 +9,7 @@ import sys import traceback import warnings +from contextlib import suppress from types import SimpleNamespace, TracebackType from typing import ( Any, @@ -80,7 +81,7 @@ BasicAuth, TimeoutHandle, ceil_timeout, - proxies_from_env, + get_env_proxy_for_url, sentinel, strip_auth_from_url, ) @@ -444,11 +445,8 @@ async def _request( if proxy is not None: proxy = URL(proxy) elif self._trust_env: - for scheme, proxy_info in proxies_from_env().items(): - if scheme == url.scheme: - proxy = proxy_info.proxy - proxy_auth = proxy_info.proxy_auth - break + with suppress(LookupError): + proxy, proxy_auth = get_env_proxy_for_url(url) req = self._request_class( method, diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 581be1ada4..2b41737675 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -40,7 +40,7 @@ cast, ) from urllib.parse import quote -from urllib.request import getproxies +from urllib.request import getproxies, proxy_bypass import async_timeout from multidict import CIMultiDict, MultiDict, MultiDictProxy @@ -269,6 +269,20 @@ def proxies_from_env() -> Dict[str, ProxyInfo]: return ret +def get_env_proxy_for_url(url: URL) -> Tuple[URL, Optional[BasicAuth]]: + """Get a permitted proxy for the given URL from the env.""" + if url.host is not None and proxy_bypass(url.host): + raise LookupError(f"Proxying is disallowed for `{url.host!r}`") + + proxies_in_env = proxies_from_env() + try: + proxy_info = proxies_in_env[url.scheme] + except KeyError: + raise LookupError(f"No proxies found for `{url!s}` in the env") + else: + return proxy_info.proxy, proxy_info.proxy_auth + + @dataclasses.dataclass(frozen=True) class MimeType: type: str diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 859ef2728c..e9b99e1217 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -6,6 +6,7 @@ import platform from math import isclose, modf from unittest import mock +from urllib.request import getproxies_environment import pytest from multidict import CIMultiDict, MultiDict @@ -515,6 +516,96 @@ def test_proxies_from_env_http_with_auth(mocker) -> None: assert proxy_auth.encoding == "latin1" +# --------------------- get_env_proxy_for_url ------------------------------ + + +@pytest.fixture +def proxy_env_vars(monkeypatch, request): + for schema in getproxies_environment().keys(): + monkeypatch.delenv(f"{schema}_proxy", False) + + for proxy_type, proxy_list in request.param.items(): + monkeypatch.setenv(proxy_type, proxy_list) + + return request.param + + +@pytest.mark.parametrize( + ("proxy_env_vars", "url_input", "expected_err_msg"), + ( + ( + {"no_proxy": "aiohttp.io"}, + "http://aiohttp.io/path", + r"Proxying is disallowed for `'aiohttp.io'`", + ), + ( + {"no_proxy": "aiohttp.io,proxy.com"}, + "http://aiohttp.io/path", + r"Proxying is disallowed for `'aiohttp.io'`", + ), + ( + {"http_proxy": "http://example.com"}, + "https://aiohttp.io/path", + r"No proxies found for `https://aiohttp.io/path` in the env", + ), + ( + {"https_proxy": "https://example.com"}, + "http://aiohttp.io/path", + r"No proxies found for `http://aiohttp.io/path` in the env", + ), + ( + {}, + "https://aiohttp.io/path", + r"No proxies found for `https://aiohttp.io/path` in the env", + ), + ( + {"https_proxy": "https://example.com"}, + "", + r"No proxies found for `` in the env", + ), + ), + indirect=["proxy_env_vars"], + ids=( + "url_matches_the_no_proxy_list", + "url_matches_the_no_proxy_list_multiple", + "url_scheme_does_not_match_http_proxy_list", + "url_scheme_does_not_match_https_proxy_list", + "no_proxies_are_set", + "url_is_empty", + ), +) +@pytest.mark.usefixtures("proxy_env_vars") +def test_get_env_proxy_for_url_negative(url_input, expected_err_msg) -> None: + url = URL(url_input) + with pytest.raises(LookupError, match=expected_err_msg): + helpers.get_env_proxy_for_url(url) + + +@pytest.mark.parametrize( + ("proxy_env_vars", "url_input"), + ( + ({"http_proxy": "http://example.com"}, "http://aiohttp.io/path"), + ({"https_proxy": "http://example.com"}, "https://aiohttp.io/path"), + ( + {"http_proxy": "http://example.com,http://proxy.org"}, + "http://aiohttp.io/path", + ), + ), + indirect=["proxy_env_vars"], + ids=( + "url_scheme_match_http_proxy_list", + "url_scheme_match_https_proxy_list", + "url_scheme_match_http_proxy_list_multiple", + ), +) +def test_get_env_proxy_for_url(proxy_env_vars, url_input) -> None: + url = URL(url_input) + proxy, proxy_auth = helpers.get_env_proxy_for_url(url) + proxy_list = proxy_env_vars[url.scheme + "_proxy"] + assert proxy == URL(proxy_list) + assert proxy_auth is None + + # ------------- set_result / set_exception ----------------------