From 2255c34a65b5b1353004dc8d49cc397cd794ec15 Mon Sep 17 00:00:00 2001 From: Darren Dormer Date: Tue, 12 Dec 2017 15:53:09 +0100 Subject: [PATCH] Fix DNS resolution by using hostname instead of netloc and strip username and password when comparing against proxy bypass items. --- AUTHORS.rst | 1 + HISTORY.rst | 3 ++- requests/utils.py | 19 +++++++++++-------- tests/test_utils.py | 25 +++++++++++++++++++++++-- 4 files changed, 37 insertions(+), 11 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 481ac6c7..2b4494ba 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -182,3 +182,4 @@ Patches and Suggestions - Arthur Vigil (`@ahvigil `_) - Nehal J Wani (`@nehaljwani `_) - Demetrios Bairaktaris (`@DemetriosBairaktaris `_) +- Darren Dormer (`@ddormer `_) diff --git a/HISTORY.rst b/HISTORY.rst index b099ecdb..db1d1f70 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -17,7 +17,8 @@ dev - Fixed issue where loading the default certificate bundle from a zip archive would raise an ``IOError`` - Fixed issue with unexpected ``ImportError`` on windows system which do not support ``winreg`` module - +- DNS resolution in proxy bypass no longer includes the username and password in + the request. This also fixes the issue of DNS queries failing on macOS. 2.18.4 (2017-08-15) +++++++++++++++++++ diff --git a/requests/utils.py b/requests/utils.py index 8c1b9bec..df18a0bf 100644 --- a/requests/utils.py +++ b/requests/utils.py @@ -703,28 +703,31 @@ def should_bypass_proxies(url, no_proxy): no_proxy_arg = no_proxy if no_proxy is None: no_proxy = get_proxy('no_proxy') - netloc = urlparse(url).netloc + parsed = urlparse(url) if no_proxy: # We need to check whether we match here. We need to see if we match - # the end of the netloc, both with and without the port. + # the end of the hostname, both with and without the port. no_proxy = ( host for host in no_proxy.replace(' ', '').split(',') if host ) - ip = netloc.split(':')[0] - if is_ipv4_address(ip): + if is_ipv4_address(parsed.hostname): for proxy_ip in no_proxy: if is_valid_cidr(proxy_ip): - if address_in_network(ip, proxy_ip): + if address_in_network(parsed.hostname, proxy_ip): return True - elif ip == proxy_ip: + elif parsed.hostname == proxy_ip: # If no_proxy ip was defined in plain IP notation instead of cidr notation & # matches the IP of the index return True else: + host_with_port = parsed.hostname + if parsed.port: + host_with_port += ':{0}'.format(parsed.port) + for host in no_proxy: - if netloc.endswith(host) or netloc.split(':')[0].endswith(host): + if parsed.hostname.endswith(host) or host_with_port.endswith(host): # The URL does match something in no_proxy, so we don't want # to apply the proxies on this URL. return True @@ -737,7 +740,7 @@ def should_bypass_proxies(url, no_proxy): # legitimate problems. with set_environ('no_proxy', no_proxy_arg): try: - bypass = proxy_bypass(netloc) + bypass = proxy_bypass(parsed.hostname) except (TypeError, socket.gaierror): bypass = False diff --git a/tests/test_utils.py b/tests/test_utils.py index 01cabe23..f39cd67b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -614,6 +614,7 @@ def test_urldefragauth(url, expected): ('http://172.16.1.1/', True), ('http://172.16.1.1:5000/', True), ('http://localhost.localdomain:5000/v1.0/', True), + ('http://google.com:6000/', True), ('http://172.16.1.12/', False), ('http://172.16.1.12:5000/', False), ('http://google.com:5000/v1.0/', False), @@ -622,11 +623,31 @@ def test_should_bypass_proxies(url, expected, monkeypatch): """Tests for function should_bypass_proxies to check if proxy can be bypassed or not """ - monkeypatch.setenv('no_proxy', '192.168.0.0/24,127.0.0.1,localhost.localdomain,172.16.1.1') - monkeypatch.setenv('NO_PROXY', '192.168.0.0/24,127.0.0.1,localhost.localdomain,172.16.1.1') + monkeypatch.setenv('no_proxy', '192.168.0.0/24,127.0.0.1,localhost.localdomain,172.16.1.1, google.com:6000') + monkeypatch.setenv('NO_PROXY', '192.168.0.0/24,127.0.0.1,localhost.localdomain,172.16.1.1, google.com:6000') assert should_bypass_proxies(url, no_proxy=None) == expected +@pytest.mark.parametrize( + 'url, expected', ( + ('http://172.16.1.1/', '172.16.1.1'), + ('http://172.16.1.1:5000/', '172.16.1.1'), + ('http://user:pass@172.16.1.1', '172.16.1.1'), + ('http://user:pass@172.16.1.1:5000', '172.16.1.1'), + ('http://hostname/', 'hostname'), + ('http://hostname:5000/', 'hostname'), + ('http://user:pass@hostname', 'hostname'), + ('http://user:pass@hostname:5000', 'hostname'), + )) +def test_should_bypass_proxies_pass_only_hostname(url, expected, mocker): + """The proxy_bypass function should be called with a hostname or IP without + a port number or auth credentials. + """ + proxy_bypass = mocker.patch('requests.utils.proxy_bypass') + should_bypass_proxies(url, no_proxy=None) + proxy_bypass.assert_called_once_with(expected) + + @pytest.mark.parametrize( 'cookiejar', ( compat.cookielib.CookieJar(),