Attachment 680889 Details for Bug 983582

[patch] Prioritize lowercase proxy variables in urllib.request

cpython-cset-49b975122022.diff (text/plain), 6.87 KB, created by Hans-Peter Jansen on 2016-06-15 11:40:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Hans-Peter Jansen

Created: 2016-06-15 11:40:11 UTC

Size: 6.87 KB

patch

obsolete

>diff -r 2c1651fbfbba -r 49b975122022 Doc/library/urllib.request.rst
>--- a/Doc/library/urllib.request.rst	Mon Apr 25 09:27:27 2016 -0500
>+++ b/Doc/library/urllib.request.rst	Mon Apr 25 08:16:23 2016 -0700
>@@ -166,6 +166,8 @@
>    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 -r 2c1651fbfbba -r 49b975122022 Lib/test/test_urllib.py
>--- a/Lib/test/test_urllib.py	Mon Apr 25 09:27:27 2016 -0500
>+++ b/Lib/test/test_urllib.py	Mon Apr 25 08:16:23 2016 -0700
>@@ -226,8 +226,46 @@
>         # 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 -r 2c1651fbfbba -r 49b975122022 Lib/urllib/request.py
>--- a/Lib/urllib/request.py	Mon Apr 25 09:27:27 2016 -0500
>+++ b/Lib/urllib/request.py	Mon Apr 25 08:16:23 2016 -0700
>@@ -2395,19 +2395,35 @@
> 
>     """
>     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 @@
> 
> 
>     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 @@
>         return 0
> 
>     def proxy_bypass(host):
>-        """Return a dictionary of scheme -> proxy server URL mappings.
>-
>-        Returns settings gathered from the environment, if specified,
>+        """Return True, if host should be bypassed.
>+
>+        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 -r 2c1651fbfbba -r 49b975122022 Misc/ACKS
>--- a/Misc/ACKS	Mon Apr 25 09:27:27 2016 -0500
>+++ b/Misc/ACKS	Mon Apr 25 08:16:23 2016 -0700
>@@ -670,6 +670,7 @@
> Bertrand Janin
> Geert Jansen
> Jack Jansen
>+Hans-Peter Jansen
> Bill Janssen
> Thomas Jarosch
> Juhana Jauhiainen
>diff -r 2c1651fbfbba -r 49b975122022 Misc/NEWS
>--- a/Misc/NEWS	Mon Apr 25 09:27:27 2016 -0500
>+++ b/Misc/NEWS	Mon Apr 25 08:16:23 2016 -0700
>@@ -107,6 +107,10 @@
> 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().

Actions: View | Diff

Attachments on bug 983582: 680889