Rev 5643: (mbp) more correct parsing of the NO_PROXY list (Martin Pool) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5643 [merge]
revision-id: pqm at pqm.ubuntu.com-20110204062039-77vw2rkha647pg2c
parent: pqm at pqm.ubuntu.com-20110203075947-l52516mt7wsqryi7
parent: v.ladeuil+lp at free.fr-20110203090725-1ej56nrwks27k8f9
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2011-02-04 06:20:39 +0000
message:
  (mbp) more correct parsing of the NO_PROXY list (Martin Pool)
modified:
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2011-01-12 22:21:05 +0000
+++ b/bzrlib/tests/test_http.py	2011-02-03 09:07:25 +0000
@@ -1079,6 +1079,11 @@
         handler.set_proxy(request, 'http')
         return request
 
+    def assertEvaluateProxyBypass(self, expected, host, no_proxy):
+        handler = _urllib2_wrappers.ProxyHandler()
+        self.assertEquals(expected,
+                          handler.evaluate_proxy_bypass(host, no_proxy))
+
     def test_empty_user(self):
         self.overrideEnv('http_proxy', 'http://bar.com')
         request = self._proxied_request()
@@ -1089,6 +1094,26 @@
         self.overrideEnv('http_proxy', 'host:1234')
         self.assertRaises(errors.InvalidURL, self._proxied_request)
 
+    def test_evaluate_proxy_bypass_true(self):
+        """The host is not proxied"""
+        self.assertEvaluateProxyBypass(True, 'example.com', 'example.com')
+        self.assertEvaluateProxyBypass(True, 'bzr.example.com', '*example.com')
+
+    def test_evaluate_proxy_bypass_false(self):
+        """The host is proxied"""
+        self.assertEvaluateProxyBypass(False, 'bzr.example.com', None)
+
+    def test_evaluate_proxy_bypass_unknown(self):
+        """The host is not explicitly proxied"""
+        self.assertEvaluateProxyBypass(None, 'example.com', 'not.example.com')
+        self.assertEvaluateProxyBypass(None, 'bzr.example.com', 'example.com')
+
+    def test_evaluate_proxy_bypass_empty_entries(self):
+        """Ignore empty entries"""
+        self.assertEvaluateProxyBypass(None, 'example.com', '')
+        self.assertEvaluateProxyBypass(None, 'example.com', ',')
+        self.assertEvaluateProxyBypass(None, 'example.com', 'foo,,bar')
+
 
 class TestProxyHttpServer(http_utils.TestCaseWithTwoWebservers):
     """Tests proxy server.

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2011-01-12 21:22:20 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2011-02-03 09:07:25 +0000
@@ -942,9 +942,31 @@
         return None
 
     def proxy_bypass(self, host):
-        """Check if host should be proxied or not"""
+        """Check if host should be proxied or not.
+
+        :returns: True to skip the proxy, False otherwise.
+        """
         no_proxy = self.get_proxy_env_var('no', default_to=None)
+        bypass = self.evaluate_proxy_bypass(host, no_proxy)
+        if bypass is None:
+            # Nevertheless, there are platform-specific ways to
+            # ignore proxies...
+            return urllib.proxy_bypass(host)
+        else:
+            return bypass
+
+    def evaluate_proxy_bypass(self, host, no_proxy):
+        """Check the host against a comma-separated no_proxy list as a string.
+
+        :param host: ``host:port`` being requested
+
+        :param no_proxy: comma-separated list of hosts to access directly.
+
+        :returns: True to skip the proxy, False not to, or None to
+            leave it to urllib.
+        """
         if no_proxy is None:
+            # All hosts are proxied
             return False
         hhost, hport = urllib.splitport(host)
         # Does host match any of the domains mentioned in
@@ -952,6 +974,9 @@
         # are fuzzy (to say the least). We try to allow most
         # commonly seen values.
         for domain in no_proxy.split(','):
+            domain = domain.strip()
+            if domain == '':
+                continue
             dhost, dport = urllib.splitport(domain)
             if hport == dport or dport is None:
                 # Protect glob chars
@@ -960,9 +985,8 @@
                 dhost = dhost.replace("?", r".")
                 if re.match(dhost, hhost, re.IGNORECASE):
                     return True
-        # Nevertheless, there are platform-specific ways to
-        # ignore proxies...
-        return urllib.proxy_bypass(host)
+        # Nothing explicitly avoid the host
+        return None
 
     def set_proxy(self, request, type):
         if self.proxy_bypass(request.get_host()):

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-01-27 17:45:24 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-02-03 05:46:08 +0000
@@ -71,6 +71,10 @@
 * ``launchpadlib`` doesn't provide the ``uris`` module in some old versions.
   (Vincent Ladeuil, #706835)
 
+* Empty entries in the ``NO_PROXY`` variable are no longer treated as matching
+  every host.
+  (Martin Pool, #586341)
+
 * Plugins incompatible with the current version of bzr no longer produce a
   warning on every command invocation.  Instead, a message is shown by
   ``bzr plugins`` and in crash reports.