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

Author: cjac

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)
 

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,

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

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
+