Merge lp:~jelmer/brz/fix-open-policy into lp:brz/3.0

[Jelmer Vernooij]

Support redirects in URL policy opener.

Support redirects in URL policy opener.

This should fix the tests for lp:~cjwatson/launchpad/use-bzr-policy-open

[Jelmer Vernooij (jelmer)]

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz-3.0/job/brz-3.0-land/43/

=== modified file 'breezy/tests/test_url_policy_open.py'
--- breezy/tests/test_url_policy_open.py	2018-11-12 01:41:38 +0000
+++ breezy/tests/test_url_policy_open.py	2019-06-09 19:10:13 +0000
@@ -30,7 +30,10 @@
     ControlDir,
     ControlDirFormat,
     )
-from ..errors import NotBranchError
+from ..errors import (
+    NotBranchError,
+    RedirectRequested,
+    )
 from ..url_policy_open import (
     BadUrl,
     _BlacklistPolicy,
@@ -44,7 +47,15 @@
     TestCase,
     TestCaseWithTransport,
     )
-from ..transport import chroot
+from ..transport import (
+    chroot,
+    get_transport,
+    register_transport_proto,
+    register_transport,
+    unregister_transport,
+    transport_list_registry,
+    Transport,
+    )
 
 
 class TestBranchOpenerCheckAndFollowBranchReference(TestCase):
@@ -305,6 +316,48 @@
             set(TrackingProber.seen_urls), {b.base, a.base})
 
 
+class TestRedirects(TestCaseWithTransport):
+
+    def setUp(self):
+        super(TestRedirects, self).setUp()
+        BranchOpener.install_hook()
+
+    def setup_redirect(self, target_url):
+        class RedirectingTransport(Transport):
+
+            def get(self, name):
+                raise RedirectRequested(self.base, target_url)
+
+            def _redirected_to(self, source, target):
+                return get_transport(target)
+
+        register_transport_proto(
+            'redirecting://', help="Test transport that redirects.")
+        register_transport('redirecting://', RedirectingTransport)
+        self.addCleanup(unregister_transport, 'redirecting://', RedirectingTransport)
+
+    def make_branch_opener(self, allowed_urls, probers=None):
+        policy = WhitelistPolicy(True, allowed_urls, True)
+        return BranchOpener(policy, probers)
+
+    def test_redirect_forbidden(self):
+        b = self.make_branch('b')
+        self.setup_redirect(b.base)
+        class TrackingProber(BzrProber):
+            seen_urls = []
+
+            @classmethod
+            def probe_transport(klass, transport):
+                klass.seen_urls.append(transport.base)
+                return BzrProber.probe_transport(transport)
+
+        opener = self.make_branch_opener(['redirecting:///'], probers=[TrackingProber])
+        self.assertRaises(BadUrl, opener.open, 'redirecting:///')
+
+        opener = self.make_branch_opener(['redirecting:///', b.base], probers=[TrackingProber])
+        opener.open('redirecting:///')
+
+
 class TestOpenOnlyScheme(TestCaseWithTransport):
     """Tests for `open_only_scheme`."""
 
 
=== modified file 'breezy/url_policy_open.py'
--- breezy/url_policy_open.py	2018-11-12 01:41:38 +0000
+++ breezy/url_policy_open.py	2019-06-09 19:10:13 +0000
@@ -22,11 +22,17 @@
 
 from . import (
     errors,
+    trace,
     urlutils,
     )
 from .branch import Branch
 from .controldir import (
     ControlDir,
+    ControlDirFormat,
+    )
+from .transport import (
+    do_catching_redirections,
+    get_transport,
     )
 
 
@@ -200,6 +206,8 @@
         """
         self.policy = policy
         self._seen_urls = set()
+        if probers is None:
+            probers = ControlDirFormat.all_probers()
         self.probers = probers
 
     @classmethod
@@ -276,15 +284,47 @@
             # spurious loop exceptions.
             self._seen_urls = set()
 
+    def _open_dir(self, url):
+        """Simple BzrDir.open clone that only uses specific probers.
+
+        :param url: URL to open
+        :return: ControlDir instance
+        """
+        def redirected(transport, e, redirection_notice):
+            self.policy.check_one_url(e.target)
+            redirected_transport = transport._redirected_to(
+                e.source, e.target)
+            if redirected_transport is None:
+                raise errors.NotBranchError(e.source)
+            trace.note(
+                '%s is%s redirected to %s',
+                transport.base, e.permanently, redirected_transport.base)
+            return redirected_transport
+
+        def find_format(transport):
+            last_error = errors.NotBranchError(transport.base)
+            for prober_kls in self.probers:
+                prober = prober_kls()
+                try:
+                    return transport, prober.probe_transport(transport)
+                except errors.NotBranchError as e:
+                    last_error = e
+            else:
+                raise last_error
+        transport = get_transport(url)
+        transport, format = do_catching_redirections(
+            find_format, transport, redirected)
+        return format.open(transport)
+
     def follow_reference(self, url):
         """Get the branch-reference value at the specified url.
 
         This exists as a separate method only to be overriden in unit tests.
         """
-        controldir = ControlDir.open(url, probers=self.probers)
+        controldir = self._open_dir(url)
         return controldir.get_branch_reference()
 
-    def open(self, url):
+    def open(self, url, ignore_fallbacks=False):
         """Open the Bazaar branch at url, first checking it.
 
         What is acceptable means is defined by the policy's `follow_reference`
@@ -295,11 +335,11 @@
 
         url = self.check_and_follow_branch_reference(url)
 
-        def open_branch(url):
-            dir = ControlDir.open(url, probers=self.probers)
-            return dir.open_branch()
+        def open_branch(url, ignore_fallbacks):
+            dir = self._open_dir(url)
+            return dir.open_branch(ignore_fallbacks=ignore_fallbacks)
         return self.run_with_transform_fallback_location_hook_installed(
-            open_branch, url)
+            open_branch, url, ignore_fallbacks)
 
 
 def open_only_scheme(allowed_scheme, url):