Issue #26804: urllib.request will prefer lower_case proxy environment variables

over UPPER_CASE or Mixed_Case ones.

Patch contributed by Hans-Peter Jansen. Reviewed by Martin Panter and Senthil Kumaran.

diff --git a/Doc/library/urllib.request.rst b/Doc/library/urllib.request.rst
index 5a47201f0528a4654aecc1fbb6e6a9fead28b4ba..8aca0ddeb21dfbd1d234aa92d0ed8e5456170eaa 100644 (file)
--- a/Doc/library/urllib.request.rst
+++ b/Doc/library/urllib.request.rst
@@ -166,6 +166,8 @@ The :mod:`urllib.request` module defines the following functions:
    in a case insensitive approach, for all operating systems first, and when it
    cannot find it, looks for proxy information from Mac OSX System
    Configuration for Mac OS X and Windows Systems Registry for Windows.
+   If both lowercase and uppercase environment variables exist (and disagree),
+   lowercase is preferred.
 
 
 The following classes are provided:

diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py
index e197a3f92eaf7aabf020ccd0221a7a70dbacd2be..3bf9ea71eee719739a780760320489df822ed0f8 100644 (file)
--- a/Lib/test/test_urllib.py
+++ b/Lib/test/test_urllib.py
@@ -226,8 +226,46 @@ class ProxyTests(unittest.TestCase):
         # getproxies_environment use lowered case truncated (no '_proxy') keys
         self.assertEqual('localhost', proxies['no'])
         # List of no_proxies with space.
-        self.env.set('NO_PROXY', 'localhost, anotherdomain.com, newdomain.com')
+        self.env.set('NO_PROXY', 'localhost, anotherdomain.com, newdomain.com:1234')
         self.assertTrue(urllib.request.proxy_bypass_environment('anotherdomain.com'))
+        self.assertTrue(urllib.request.proxy_bypass_environment('anotherdomain.com:8888'))
+        self.assertTrue(urllib.request.proxy_bypass_environment('newdomain.com:1234'))
+
+
+class ProxyTests_withOrderedEnv(unittest.TestCase):
+
+    def setUp(self):
+        # We need to test conditions, where variable order _is_ significant
+        self._saved_env = os.environ
+        # Monkey patch os.environ, start with empty fake environment
+        os.environ = collections.OrderedDict()
+
+    def tearDown(self):
+        os.environ = self._saved_env
+
+    def test_getproxies_environment_prefer_lowercase(self):
+        # Test lowercase preference with removal
+        os.environ['no_proxy'] = ''
+        os.environ['No_Proxy'] = 'localhost'
+        self.assertFalse(urllib.request.proxy_bypass_environment('localhost'))
+        self.assertFalse(urllib.request.proxy_bypass_environment('arbitrary'))
+        os.environ['http_proxy'] = ''
+        os.environ['HTTP_PROXY'] = 'http://somewhere:3128'
+        proxies = urllib.request.getproxies_environment()
+        self.assertEqual({}, proxies)
+        # Test lowercase preference of proxy bypass and correct matching including ports
+        os.environ['no_proxy'] = 'localhost, noproxy.com, my.proxy:1234'
+        os.environ['No_Proxy'] = 'xyz.com'
+        self.assertTrue(urllib.request.proxy_bypass_environment('localhost'))
+        self.assertTrue(urllib.request.proxy_bypass_environment('noproxy.com:5678'))
+        self.assertTrue(urllib.request.proxy_bypass_environment('my.proxy:1234'))
+        self.assertFalse(urllib.request.proxy_bypass_environment('my.proxy'))
+        self.assertFalse(urllib.request.proxy_bypass_environment('arbitrary'))
+        # Test lowercase preference with replacement
+        os.environ['http_proxy'] = 'http://somewhere:3128'
+        os.environ['Http_Proxy'] = 'http://somewhereelse:3128'
+        proxies = urllib.request.getproxies_environment()
+        self.assertEqual('http://somewhere:3128', proxies['http'])
 
 class urlopen_HttpTests(unittest.TestCase, FakeHTTPMixin, FakeFTPMixin):
     """Test urlopen() opening a fake http connection."""

diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
index fc8ef7f91b09fbb8ce59827ff6301c0153eef9d1..fc42d49a9f3da26859e4376b9f56834a2229ceea 100644 (file)
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -2395,19 +2395,35 @@ def getproxies_environment():
 
     """
     proxies = {}
+    # in order to prefer lowercase variables, process environment in
+    # two passes: first matches any, second pass matches lowercase only
     for name, value in os.environ.items():
         name = name.lower()
         if value and name[-6:] == '_proxy':
             proxies[name[:-6]] = value
+    for name, value in os.environ.items():
+        if name[-6:] == '_proxy':
+            name = name.lower()
+            if value:
+                proxies[name[:-6]] = value
+            else:
+                proxies.pop(name[:-6], None)
     return proxies
 
-def proxy_bypass_environment(host):
+def proxy_bypass_environment(host, proxies=None):
     """Test if proxies should not be used for a particular host.
 
-    Checks the environment for a variable named no_proxy, which should
-    be a list of DNS suffixes separated by commas, or '*' for all hosts.
+    Checks the proxy dict for the value of no_proxy, which should
+    be a list of comma separated DNS suffixes, or '*' for all hosts.
+
     """
-    no_proxy = os.environ.get('no_proxy', '') or os.environ.get('NO_PROXY', '')
+    if proxies is None:
+        proxies = getproxies_environment()
+    # don't bypass, if no_proxy isn't specified
+    try:
+        no_proxy = proxies['no']
+    except KeyError:
+        return 0
     # '*' is special case for always bypass
     if no_proxy == '*':
         return 1
@@ -2502,8 +2518,15 @@ if sys.platform == 'darwin':
 
 
     def proxy_bypass(host):
-        if getproxies_environment():
-            return proxy_bypass_environment(host)
+        """Return True, if host should be bypassed.
+
+        Checks proxy settings gathered from the environment, if specified,
+        or from the MacOSX framework SystemConfiguration.
+
+        """
+        proxies = getproxies_environment()
+        if proxies:
+            return proxy_bypass_environment(host, proxies)
         else:
             return proxy_bypass_macosx_sysconf(host)
 
@@ -2617,14 +2640,15 @@ elif os.name == 'nt':
         return 0
 
     def proxy_bypass(host):
-        """Return a dictionary of scheme -> proxy server URL mappings.
+        """Return True, if host should be bypassed.
 
-        Returns settings gathered from the environment, if specified,
+        Checks proxy settings gathered from the environment, if specified,
         or the registry.
 
         """
-        if getproxies_environment():
-            return proxy_bypass_environment(host)
+        proxies = getproxies_environment()
+        if proxies:
+            return proxy_bypass_environment(host, proxies)
         else:
             return proxy_bypass_registry(host)
 

diff --git a/Misc/ACKS b/Misc/ACKS
index 6f4bfdbb6b3cc4e2341eaa8bfc4a86697ea773a0..2d920dbdcde21047698bf7a8671073daa542f36f 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -670,6 +670,7 @@ Kjetil Jacobsen
 Bertrand Janin
 Geert Jansen
 Jack Jansen
+Hans-Peter Jansen
 Bill Janssen
 Thomas Jarosch
 Juhana Jauhiainen

diff --git a/Misc/NEWS b/Misc/NEWS
index 20dc5197e650bda58d128450b969e8844ad86f1a..6e9ed109d234251df600600f084882f895a720a9 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -107,6 +107,10 @@ Core and Builtins
 Library
 -------
 
+- Issue #26804: urllib.request will prefer lower_case proxy environment
+  variables over UPPER_CASE or Mixed_Case ones. Patch contributed by Hans-Peter
+  Jansen.
+
 - Issue #26837: assertSequenceEqual() now correctly outputs non-stringified
   differing items (like bytes in the -b mode).  This affects assertListEqual()
   and assertTupleEqual().