diff --git a/CHANGES.rst b/CHANGES.rst index f750371b70..b5dde2150a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,11 @@ Changes in Apache Libcloud 4.0.0 Common ~~~~~~ +- Respect the ``no_proxy`` / ``NO_PROXY`` environment variable so an explicitly + configured proxy is bypassed for matching hosts. + (GITHUB-2077) + [Sanjay Santhanam - @Sanjays2402] + - Move tests to python 3.12. (#2152) [Miguel Caballer - @micafer] diff --git a/libcloud/http.py b/libcloud/http.py index 4c1c1b01e4..f6dc59fd0c 100644 --- a/libcloud/http.py +++ b/libcloud/http.py @@ -21,11 +21,9 @@ import warnings import requests +from requests.utils import should_bypass_proxies from requests.adapters import HTTPAdapter -import libcloud.security -from libcloud.utils.py3 import urlparse - try: # requests no longer vendors urllib3 in newer versions # https://github.com/python/typeshed/issues/6893#issuecomment-1012511758 @@ -33,6 +31,8 @@ except ImportError: from requests.packages.urllib3.poolmanager import PoolManager # type: ignore +import libcloud.security +from libcloud.utils.py3 import urlparse __all__ = ["LibcloudBaseConnection", "LibcloudConnection"] @@ -112,6 +112,32 @@ def set_http_proxy(self, proxy_url): "https": proxy_url, } + def _proxies_for_url(self, url): + """ + Return the proxy mapping to use for ``url``. + + An explicitly configured proxy is skipped when the target host matches + the ``no_proxy`` / ``NO_PROXY`` environment variable, so libcloud + behaves consistently with other HTTP clients. + + :param url: Absolute request URL. + :type url: ``str`` + + :rtype: ``dict`` or ``None`` + """ + if not self.session.proxies: + return None + + if should_bypass_proxies(url, no_proxy=None): + # Explicitly disable the configured schemes. Returning {} is not + # enough: requests merges per-request proxies with the session + # proxies, so the session-level proxy would be merged back in. + # ``None`` values are stripped by requests' merge, leaving an + # empty effective mapping. + return {"http": None, "https": None} + + return None + def _parse_proxy_url(self, proxy_url): """ Parse and validate a proxy URL. @@ -231,6 +257,7 @@ def request(self, method, url, body=None, headers=None, raw=False, stream=False, verify=self.verification, timeout=self.session.timeout, hooks=hooks, + proxies=self._proxies_for_url(url), ) def prepared_request(self, method, url, body=None, headers=None, raw=False, stream=False): diff --git a/libcloud/test/test_connection.py b/libcloud/test/test_connection.py index a7bf1d7c8e..d23fa7a829 100644 --- a/libcloud/test/test_connection.py +++ b/libcloud/test/test_connection.py @@ -21,7 +21,7 @@ from unittest.mock import Mock, patch import requests_mock -from requests.adapters import HTTPAdapter +from requests.adapters import HTTPAdapter, select_proxy from requests.exceptions import ConnectTimeout import libcloud.common.base @@ -161,6 +161,112 @@ def test_constructor(self): {"http": "https://127.0.0.6:3129", "https": "https://127.0.0.6:3129"}, ) + def test_proxy_is_bypassed_for_no_proxy_hosts(self): + # Regression test for GITHUB-2077: an explicitly configured proxy must + # not be used for hosts listed in the no_proxy environment variable. + old_no_proxy = os.environ.get("no_proxy") + old_NO_PROXY = os.environ.get("NO_PROXY") + os.environ["no_proxy"] = "internal.example.com" + # Pin NO_PROXY too: an externally-set uppercase variable must not leak + # into this test, and our addCleanup must restore (not drop) whatever + # was there before, since it runs after tearDown(). + os.environ.pop("NO_PROXY", None) + + def restore_proxy_env(): + for name, value in ( + ("no_proxy", old_no_proxy), + ("NO_PROXY", old_NO_PROXY), + ): + if value is None: + os.environ.pop(name, None) + else: + os.environ[name] = value + + self.addCleanup(restore_proxy_env) + + conn = LibcloudConnection(host="internal.example.com", port=443) + conn.set_http_proxy("http://proxy.example.com:3128") + + self.assertEqual( + conn._proxies_for_url("https://internal.example.com/path"), + {"http": None, "https": None}, + ) + self.assertIsNone(conn._proxies_for_url("https://other.example.com/path")) + + def test_request_effective_proxies_bypass_session_proxy_for_no_proxy_host(self): + # End-to-end-ish check for GITHUB-2077: verify the proxy mapping that + # actually reaches the HTTP adapter after requests merges the + # per-request proxies with the session proxies. Returning {} from + # _proxies_for_url() is not sufficient because requests merges the + # session-level proxies back in, so the schemes must be explicitly + # disabled with None. + old_no_proxy = os.environ.get("no_proxy") + old_NO_PROXY = os.environ.get("NO_PROXY") + os.environ["no_proxy"] = "internal.example.com" + os.environ.pop("NO_PROXY", None) + # Pin the proxy environment too: ambient http_proxy/https_proxy would + # otherwise be merged in by requests (trust_env) and break the + # control-case assertions below. + old_proxy_env = {} + for name in ( + "http_proxy", + "https_proxy", + "all_proxy", + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + ): + old_proxy_env[name] = os.environ.pop(name, None) + + def restore_proxy_env(): + for name, value in ( + ("no_proxy", old_no_proxy), + ("NO_PROXY", old_NO_PROXY), + ): + if value is None: + os.environ.pop(name, None) + else: + os.environ[name] = value + for name, value in old_proxy_env.items(): + if value is None: + os.environ.pop(name, None) + else: + os.environ[name] = value + + self.addCleanup(restore_proxy_env) + + captured = {} + + def mock_send(self, request, **kwargs): + captured["proxies"] = kwargs.get("proxies", {}) + captured["url"] = request.url + mock_response = Mock() + mock_response.status_code = 200 + mock_response.headers = {"content-type": "application/json", "location": ""} + mock_response.text = "OK" + mock_response.history = [] # No redirects + return mock_response + + with patch.object(HTTPAdapter, "send", mock_send): + conn = LibcloudConnection(host="internal.example.com", port=443) + conn.set_http_proxy("http://proxy.example.com:3128") + conn.request("GET", "/path") + + # The session proxy must not leak back in via requests' merge. + self.assertNotIn("http://proxy.example.com:3128", captured["proxies"].values()) + self.assertIsNone(select_proxy(captured["url"], captured["proxies"])) + + # Control: a host that is not bypassed still uses the proxy. + conn = LibcloudConnection(host="other.example.com", port=443) + conn.set_http_proxy("http://proxy.example.com:3128") + conn.request("GET", "/path") + + self.assertEqual(captured["proxies"].get("http"), "http://proxy.example.com:3128") + self.assertEqual( + select_proxy(captured["url"], captured["proxies"]), + "http://proxy.example.com:3128", + ) + def test_proxy_environment_variables_respected(self): """ Test that proxy environment variables are respected by the underlying Requests library