From a784de5a5209aabc9c64aae809a9ae5caa706b5b Mon Sep 17 00:00:00 2001 From: "C.J. Collier" Date: Fri, 24 Oct 2025 20:31:33 +0000 Subject: [PATCH] Fix: Respect no_proxy in proxies dictionary and NO_PROXY env var This commit addresses issue #5000 by ensuring that the `no_proxy` directive is respected, whether it's provided directly within the `proxies` dictionary passed to request functions or set as the `NO_PROXY` environment variable. Previously, `no_proxy` in the `proxies` dictionary was not fully honored, and the `NO_PROXY` environment variable was not always checked when a `proxies` dictionary was provided. This change includes: 1. **`src/requests/sessions.py`**: Modified the `Session.send` method to check the `no_proxy` key within the `kwargs['proxies']` dictionary. If the request URL matches any pattern in the `no_proxy` list, the proxies are cleared for that request. 2. **`src/requests/utils.py`**: Updated the `select_proxy` function to check the `NO_PROXY` environment variable using `should_bypass_proxies` before selecting a proxy from the provided `proxies` dictionary. 3. **`tests/test_requests.py`**: Added new test cases (`test_no_proxy_in_proxies_dict`, `test_no_proxy_star_in_proxies_dict`, `test_no_proxy_not_matching_in_proxies_dict`) to verify that `no_proxy` within the `proxies` dictionary works as expected, using mocks to check if the proxy is bypassed. 4. **`tests/test_utils.py`**: Added new test cases (`test_select_proxy_with_no_proxy`) to ensure the `NO_PROXY` environment variable is correctly handled by `select_proxy`. These changes ensure consistent behavior for proxy bypass logic, regardless of how the proxy settings are configured. Closes #5000 --- src/requests/sessions.py | 6 ++++ src/requests/utils.py | 4 +++ tests/test_requests.py | 64 ++++++++++++++++++++++++++++++++++++++++ tests/test_utils.py | 28 ++++++++++++++++++ 4 files changed, 102 insertions(+) diff --git a/src/requests/sessions.py b/src/requests/sessions.py index 731550de88..c9b3f97342 100644 --- a/src/requests/sessions.py +++ b/src/requests/sessions.py @@ -699,6 +699,12 @@ def send(self, request, **kwargs): # Start time (approximately) of the request start = preferred_clock() + # Check if the URL should bypass the proxy based on 'no_proxy' in kwargs + if kwargs.get("proxies"): + no_proxy_list = kwargs["proxies"].get("no_proxy") + if should_bypass_proxies(request.url, no_proxy=no_proxy_list): + kwargs["proxies"] = {} # Clear proxies if URL should be bypassed + # Send the request r = adapter.send(request, **kwargs) diff --git a/src/requests/utils.py b/src/requests/utils.py index 8ab55852cc..977b46ed76 100644 --- a/src/requests/utils.py +++ b/src/requests/utils.py @@ -836,6 +836,10 @@ def select_proxy(url, proxies): if urlparts.hostname is None: return proxies.get(urlparts.scheme, proxies.get("all")) + # Check NO_PROXY from environment first + if should_bypass_proxies(url, no_proxy=os.environ.get('NO_PROXY')): + return None + proxy_keys = [ urlparts.scheme + "://" + urlparts.hostname, urlparts.scheme, diff --git a/tests/test_requests.py b/tests/test_requests.py index 75d2deff2e..f500b7ee46 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -1997,7 +1997,71 @@ def test_rewind_partially_read_body(self): requests.utils.rewind_body(prep) assert prep.body.read() == b"data" + def test_rewind_body_no_seek(self): + pass + + def test_no_proxy_in_proxies_dict(self, httpbin): + # Test case 1: 'no_proxy' in proxies dict + proxies = {"https": "https://b.r.o.k.e.n.com", "no_proxy": "google.com"} + with mock.patch('requests.adapters.HTTPAdapter.send') as mock_send: + mock_response = mock.Mock() + mock_response.url = "https://google.com" + mock_response.headers = {} + mock_response.status_code = 200 + mock_response.is_redirect = False + mock_response.history = [] + mock_send.return_value = mock_response + requests.get("https://google.com", proxies=proxies) + # Check if proxies were cleared before sending + args, kwargs = mock_send.call_args + assert not kwargs.get("proxies") + + # Test case 2: 'no_proxy' in proxies dict with env var + proxies = {"https": "https://b.r.o.k.e.n.com"} + with override_environ(NO_PROXY="google.com"): + with mock.patch('requests.adapters.HTTPAdapter.send') as mock_send: + mock_response = mock.Mock() + mock_response.url = "https://google.com" + mock_response.headers = {} + mock_response.status_code = 200 + mock_response.is_redirect = False + mock_response.history = [] + mock_send.return_value = mock_response + requests.get("https://google.com", proxies=proxies) + # Check if proxies were cleared before sending + args, kwargs = mock_send.call_args + assert not kwargs.get("proxies") + + def test_no_proxy_star_in_proxies_dict(self, httpbin): + proxies = {"https": "https://b.r.o.k.e.n.com", "no_proxy": "*"} + with mock.patch('requests.adapters.HTTPAdapter.send') as mock_send: + mock_response = mock.Mock() + mock_response.url = "https://google.com" + mock_response.headers = {} + mock_response.status_code = 200 + mock_response.is_redirect = False + mock_response.history = [] + mock_send.return_value = mock_response + requests.get("https://google.com", proxies=proxies) + # Check if proxies were cleared before sending + args, kwargs = mock_send.call_args + assert not kwargs.get("proxies") + + def test_no_proxy_not_matching_in_proxies_dict(self, httpbin): + proxies = {"https": "https://b.r.o.k.e.n.com", "no_proxy": "example.com"} + with mock.patch('requests.adapters.HTTPAdapter.send') as mock_send: + mock_response = mock.Mock() + mock_response.url = "https://google.com" + mock_response.headers = {} + mock_response.status_code = 200 + mock_response.is_redirect = False + mock_response.history = [] + mock_send.return_value = mock_response + requests.get("https://google.com", proxies=proxies) + # Check if proxies were NOT cleared before sending + args, kwargs = mock_send.call_args + assert kwargs.get("proxies") == proxies class BadFileObj: def __init__(self, data): self.data = data diff --git a/tests/test_utils.py b/tests/test_utils.py index f9a287af1b..864801a19d 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -975,3 +975,31 @@ def QueryValueEx(key, value_name): monkeypatch.setattr(winreg, "OpenKey", OpenKey) monkeypatch.setattr(winreg, "QueryValueEx", QueryValueEx) assert should_bypass_proxies("http://example.com/", None) is False + + + +@pytest.mark.parametrize( + "url, no_proxy_env, proxies, expected_proxy", + [ + ("http://example.com", "example.com", {"http": "http://proxy.com"}, None), + ("http://example.com", "other.com", {"http": "http://proxy.com"}, "http://proxy.com"), + ("http://test.example.com", "example.com", {"http": "http://proxy.com"}, None), + ("http://example.com", "*", {"http": "http://proxy.com"}, None), + ("http://example.com", None, {"http": "http://proxy.com"}, "http://proxy.com"), + ("http://internal.com", "internal.com", {"all": "http://proxy.com"}, None), + ("https://internal.com", "internal.com", {"all": "http://proxy.com"}, None), + ], +) +def test_select_proxy_with_no_proxy( + url, no_proxy_env, proxies, expected_proxy, monkeypatch +): + """Test that select_proxy honors NO_PROXY environment variable.""" + if no_proxy_env: + monkeypatch.setenv("NO_PROXY", no_proxy_env) + else: + monkeypatch.delenv("NO_PROXY", raising=False) + + assert select_proxy(url, proxies) == expected_proxy + +# EOF Marker +