Rev 5641: Add tests and comments to clarify the feature. in file:///home/vila/src/bzr/reviews/586341-noproxy/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/reviews/586341-noproxy/

------------------------------------------------------------
revno: 5641
revision-id: v.ladeuil+lp at free.fr-20110203090725-1ej56nrwks27k8f9
parent: mbp at canonical.com-20110203054608-3r779z0kq7cewtal
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 586341-noproxy
timestamp: Thu 2011-02-03 10:07:25 +0100
message:
  Add tests and comments to clarify the feature.
-------------- next part --------------
=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2011-02-03 05:46:08 +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,15 +1094,25 @@
         self.overrideEnv('http_proxy', 'host:1234')
         self.assertRaises(errors.InvalidURL, self._proxied_request)
 
-    def test_evaluate_proxy_bypass(self):
-        """Test matching of no_proxy etc"""
-        handler = _urllib2_wrappers.ProxyHandler()
-        # https://bugs.launchpad.net/bzr/+bug/586341
-        self.assertEquals(
-            None,
-            handler.evaluate_proxy_bypass(
-                'example.com',
-                ','))
+    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):

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2011-02-03 05:46:08 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2011-02-03 09:07:25 +0000
@@ -942,25 +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)
-        result = self.evaluate_proxy_bypass(host, no_proxy)
-        if result is 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 result
+            return bypass
 
     def evaluate_proxy_bypass(self, host, no_proxy):
-        """Check the host against a comma-separated no_proxy list.
-
-        :param host: host:port being requested
+        """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
@@ -978,7 +984,9 @@
                 dhost = dhost.replace("*", r".*")
                 dhost = dhost.replace("?", r".")
                 if re.match(dhost, hhost, re.IGNORECASE):
-                    return True        
+                    return True
+        # Nothing explicitly avoid the host
+        return None
 
     def set_proxy(self, request, type):
         if self.proxy_bypass(request.get_host()):