launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04505
[Merge] lp:~jelmer/launchpad/safe-branch-open into lp:launchpad
Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/safe-branch-open into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jelmer/launchpad/safe-branch-open/+merge/70654
Extract a generic "SafeBranchOpener" class from the BranchMirrorer code in lp.codehosting.puller.
The related policy class "BranchPolicy" which determines what branches are safe to open and how to do stacking for target branches has been split as well - there now is a SafeBranchOpener-specific "BranchOpenPolicy", and a subclass of that ("BranchMirrorPolicy") which also has some methods for getting the stacked branch.
The next step is to eliminate the other safe open function in lp.codehosting.bzrutils in favor of this one (bug 820453).
--
https://code.launchpad.net/~jelmer/launchpad/safe-branch-open/+merge/70654
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/safe-branch-open into lp:launchpad.
=== modified file 'lib/lp/codehosting/puller/tests/__init__.py'
--- lib/lp/codehosting/puller/tests/__init__.py 2011-06-02 11:16:47 +0000
+++ lib/lp/codehosting/puller/tests/__init__.py 2011-08-06 17:48:46 +0000
@@ -29,73 +29,11 @@
PullerWorkerProtocol,
)
from lp.codehosting.tests.helpers import LoomTestMixin
+from lp.codehosting.safe_open import AcceptAnythingPolicy
from lp.codehosting.vfs import branch_id_to_path
-from lp.codehosting.vfs.branchfs import (
- BadUrl,
- BranchPolicy,
- )
from lp.testing import TestCaseWithFactory
-class BlacklistPolicy(BranchPolicy):
- """Branch policy that forbids certain URLs."""
-
- def __init__(self, should_follow_references, unsafe_urls=None):
- if unsafe_urls is None:
- unsafe_urls = set()
- self._unsafe_urls = unsafe_urls
- self._should_follow_references = should_follow_references
-
- def shouldFollowReferences(self):
- return self._should_follow_references
-
- def checkOneURL(self, url):
- if url in self._unsafe_urls:
- raise BadUrl(url)
-
- def transformFallbackLocation(self, branch, url):
- """See `BranchPolicy.transformFallbackLocation`.
-
- This class is not used for testing our smarter stacking features so we
- just do the simplest thing: return the URL that would be used anyway
- and don't check it.
- """
- return urlutils.join(branch.base, url), False
-
-
-class AcceptAnythingPolicy(BlacklistPolicy):
- """Accept anything, to make testing easier."""
-
- def __init__(self):
- super(AcceptAnythingPolicy, self).__init__(True, set())
-
-
-class WhitelistPolicy(BranchPolicy):
- """Branch policy that only allows certain URLs."""
-
- def __init__(self, should_follow_references, allowed_urls=None,
- check=False):
- if allowed_urls is None:
- allowed_urls = []
- self.allowed_urls = set(url.rstrip('/') for url in allowed_urls)
- self.check = check
-
- def shouldFollowReferences(self):
- return self._should_follow_references
-
- def checkOneURL(self, url):
- if url.rstrip('/') not in self.allowed_urls:
- raise BadUrl(url)
-
- def transformFallbackLocation(self, branch, url):
- """See `BranchPolicy.transformFallbackLocation`.
-
- Here we return the URL that would be used anyway and optionally check
- it.
- """
- return urlutils.join(branch.base, url), self.check
-
-
class PullerWorkerMixin:
"""Mixin for tests that want to make PullerWorker objects.
=== modified file 'lib/lp/codehosting/puller/tests/test_errors.py'
--- lib/lp/codehosting/puller/tests/test_errors.py 2010-08-20 20:31:18 +0000
+++ lib/lp/codehosting/puller/tests/test_errors.py 2011-08-06 17:48:46 +0000
@@ -23,17 +23,15 @@
from lp.code.enums import BranchType
from lp.codehosting.puller.worker import (
+ BadUrlLaunchpad,
+ BadUrlScheme,
+ BadUrlSsh,
BranchLoopError,
BranchMirrorer,
BranchReferenceForbidden,
PullerWorker,
PullerWorkerProtocol,
)
-from lp.codehosting.vfs.branchfs import (
- BadUrlLaunchpad,
- BadUrlScheme,
- BadUrlSsh,
- )
class StubbedPullerWorkerProtocol(PullerWorkerProtocol):
=== modified file 'lib/lp/codehosting/puller/tests/test_worker.py'
--- lib/lp/codehosting/puller/tests/test_worker.py 2010-10-15 08:47:20 +0000
+++ lib/lp/codehosting/puller/tests/test_worker.py 2011-08-06 17:48:46 +0000
@@ -14,18 +14,15 @@
import bzrlib.branch
from bzrlib.branch import (
BranchReferenceFormat,
- BzrBranchFormat7,
)
from bzrlib.bzrdir import (
BzrDir,
- BzrDirMetaFormat1,
)
from bzrlib.errors import (
IncompatibleRepositories,
NotBranchError,
NotStacked,
)
-from bzrlib.repofmt.pack_repo import RepositoryFormatKnitPack1
from bzrlib.revision import NULL_REVISION
from bzrlib.tests import (
TestCaseInTempDir,
@@ -35,28 +32,24 @@
from lp.code.enums import BranchType
from lp.codehosting.puller.tests import (
- AcceptAnythingPolicy,
- BlacklistPolicy,
FixedHttpServer,
PullerWorkerMixin,
- WhitelistPolicy,
)
from lp.codehosting.puller.worker import (
- BranchLoopError,
- BranchMirrorer,
- BranchReferenceForbidden,
- install_worker_ui_factory,
- PullerWorkerProtocol,
- WORKER_ACTIVITY_NETWORK,
- )
-from lp.codehosting.vfs.branchfs import (
- BadUrl,
BadUrlLaunchpad,
BadUrlScheme,
BadUrlSsh,
- BranchPolicy,
ImportedBranchPolicy,
+ install_worker_ui_factory,
MirroredBranchPolicy,
+ PullerWorkerProtocol,
+ SafeBranchOpener,
+ WORKER_ACTIVITY_NETWORK,
+ )
+from lp.codehosting.safe_open import (
+ AcceptAnythingPolicy,
+ BadUrl,
+ BranchOpenPolicy,
)
from lp.testing import TestCase
from lp.testing.factory import (
@@ -280,199 +273,8 @@
self.assertEqual('', stacked_on_url)
-class TestBranchMirrorerCheckAndFollowBranchReference(TestCase):
- """Unit tests for `BranchMirrorer.checkAndFollowBranchReference`."""
-
- class StubbedBranchMirrorer(BranchMirrorer):
- """BranchMirrorer that provides canned answers.
-
- We implement the methods we need to to be able to control all the
- inputs to the `BranchMirrorer.checkSource` method, which is what is
- being tested in this class.
- """
-
- def __init__(self, references, policy):
- parent_cls = TestBranchMirrorerCheckAndFollowBranchReference
- super(parent_cls.StubbedBranchMirrorer, self).__init__(policy)
- self._reference_values = {}
- for i in range(len(references) - 1):
- self._reference_values[references[i]] = references[i+1]
- self.follow_reference_calls = []
-
- def followReference(self, url):
- self.follow_reference_calls.append(url)
- return self._reference_values[url]
-
- def makeBranchMirrorer(self, should_follow_references, references,
- unsafe_urls=None):
- policy = BlacklistPolicy(should_follow_references, unsafe_urls)
- opener = self.StubbedBranchMirrorer(references, policy)
- return opener
-
- def testCheckInitialURL(self):
- # checkSource rejects all URLs that are not allowed.
- opener = self.makeBranchMirrorer(None, [], set(['a']))
- self.assertRaises(BadUrl, opener.checkAndFollowBranchReference, 'a')
-
- def testNotReference(self):
- # When branch references are forbidden, checkAndFollowBranchReference
- # does not raise on non-references.
- opener = self.makeBranchMirrorer(False, ['a', None])
- self.assertEquals('a', opener.checkAndFollowBranchReference('a'))
- self.assertEquals(['a'], opener.follow_reference_calls)
-
- def testBranchReferenceForbidden(self):
- # checkAndFollowBranchReference raises BranchReferenceForbidden if
- # branch references are forbidden and the source URL points to a
- # branch reference.
- opener = self.makeBranchMirrorer(False, ['a', 'b'])
- self.assertRaises(
- BranchReferenceForbidden,
- opener.checkAndFollowBranchReference, 'a')
- self.assertEquals(['a'], opener.follow_reference_calls)
-
- def testAllowedReference(self):
- # checkAndFollowBranchReference does not raise if following references
- # is allowed and the source URL points to a branch reference to a
- # permitted location.
- opener = self.makeBranchMirrorer(True, ['a', 'b', None])
- self.assertEquals('b', opener.checkAndFollowBranchReference('a'))
- self.assertEquals(['a', 'b'], opener.follow_reference_calls)
-
- def testCheckReferencedURLs(self):
- # checkAndFollowBranchReference checks if the URL a reference points
- # to is safe.
- opener = self.makeBranchMirrorer(
- True, ['a', 'b', None], unsafe_urls=set('b'))
- self.assertRaises(BadUrl, opener.checkAndFollowBranchReference, 'a')
- self.assertEquals(['a'], opener.follow_reference_calls)
-
- def testSelfReferencingBranch(self):
- # checkAndFollowBranchReference raises BranchReferenceLoopError if
- # following references is allowed and the source url points to a
- # self-referencing branch reference.
- opener = self.makeBranchMirrorer(True, ['a', 'a'])
- self.assertRaises(
- BranchLoopError, opener.checkAndFollowBranchReference, 'a')
- self.assertEquals(['a'], opener.follow_reference_calls)
-
- def testBranchReferenceLoop(self):
- # checkAndFollowBranchReference raises BranchReferenceLoopError if
- # following references is allowed and the source url points to a loop
- # of branch references.
- references = ['a', 'b', 'a']
- opener = self.makeBranchMirrorer(True, references)
- self.assertRaises(
- BranchLoopError, opener.checkAndFollowBranchReference, 'a')
- self.assertEquals(['a', 'b'], opener.follow_reference_calls)
-
-
-class TestBranchMirrorerStacking(TestCaseWithTransport):
-
- def makeBranchMirrorer(self, allowed_urls):
- policy = WhitelistPolicy(True, allowed_urls, True)
- return BranchMirrorer(policy)
-
- def makeBranch(self, path, branch_format, repository_format):
- """Make a Bazaar branch at 'path' with the given formats."""
- bzrdir_format = BzrDirMetaFormat1()
- bzrdir_format.set_branch_format(branch_format)
- bzrdir = self.make_bzrdir(path, format=bzrdir_format)
- repository_format.initialize(bzrdir)
- return bzrdir.create_branch()
-
- def testAllowedURL(self):
- # checkSource does not raise an exception for branches stacked on
- # branches with allowed URLs.
- stacked_on_branch = self.make_branch('base-branch', format='1.6')
- stacked_branch = self.make_branch('stacked-branch', format='1.6')
- stacked_branch.set_stacked_on_url(stacked_on_branch.base)
- opener = self.makeBranchMirrorer(
- [stacked_branch.base, stacked_on_branch.base])
- # This doesn't raise an exception.
- opener.open(stacked_branch.base)
-
- def testUnstackableRepository(self):
- # checkSource treats branches with UnstackableRepositoryFormats as
- # being not stacked.
- branch = self.makeBranch(
- 'unstacked', BzrBranchFormat7(), RepositoryFormatKnitPack1())
- opener = self.makeBranchMirrorer([branch.base])
- # This doesn't raise an exception.
- opener.open(branch.base)
-
- def testAllowedRelativeURL(self):
- # checkSource passes on absolute urls to checkOneURL, even if the
- # value of stacked_on_location in the config is set to a relative URL.
- stacked_on_branch = self.make_branch('base-branch', format='1.6')
- stacked_branch = self.make_branch('stacked-branch', format='1.6')
- stacked_branch.set_stacked_on_url('../base-branch')
- opener = self.makeBranchMirrorer(
- [stacked_branch.base, stacked_on_branch.base])
- # Note that stacked_on_branch.base is not '../base-branch', it's an
- # absolute URL.
- self.assertNotEqual('../base-branch', stacked_on_branch.base)
- # This doesn't raise an exception.
- opener.open(stacked_branch.base)
-
- def testAllowedRelativeNested(self):
- # Relative URLs are resolved relative to the stacked branch.
- self.get_transport().mkdir('subdir')
- a = self.make_branch('subdir/a', format='1.6')
- b = self.make_branch('b', format='1.6')
- b.set_stacked_on_url('../subdir/a')
- c = self.make_branch('subdir/c', format='1.6')
- c.set_stacked_on_url('../../b')
- opener = self.makeBranchMirrorer([c.base, b.base, a.base])
- # This doesn't raise an exception.
- opener.open(c.base)
-
- def testForbiddenURL(self):
- # checkSource raises a BadUrl exception if a branch is stacked on a
- # branch with a forbidden URL.
- stacked_on_branch = self.make_branch('base-branch', format='1.6')
- stacked_branch = self.make_branch('stacked-branch', format='1.6')
- stacked_branch.set_stacked_on_url(stacked_on_branch.base)
- opener = self.makeBranchMirrorer([stacked_branch.base])
- self.assertRaises(BadUrl, opener.open, stacked_branch.base)
-
- def testForbiddenURLNested(self):
- # checkSource raises a BadUrl exception if a branch is stacked on a
- # branch that is in turn stacked on a branch with a forbidden URL.
- a = self.make_branch('a', format='1.6')
- b = self.make_branch('b', format='1.6')
- b.set_stacked_on_url(a.base)
- c = self.make_branch('c', format='1.6')
- c.set_stacked_on_url(b.base)
- opener = self.makeBranchMirrorer([c.base, b.base])
- self.assertRaises(BadUrl, opener.open, c.base)
-
- def testSelfStackedBranch(self):
- # checkSource raises StackingLoopError if a branch is stacked on
- # itself. This avoids infinite recursion errors.
- a = self.make_branch('a', format='1.6')
- # Bazaar 1.17 and up make it harder to create branches like this.
- # It's still worth testing that we don't blow up in the face of them,
- # so we grovel around a bit to create one anyway.
- a.get_config().set_user_option('stacked_on_location', a.base)
- opener = self.makeBranchMirrorer([a.base])
- self.assertRaises(BranchLoopError, opener.open, a.base)
-
- def testLoopStackedBranch(self):
- # checkSource raises StackingLoopError if a branch is stacked in such
- # a way so that it is ultimately stacked on itself. e.g. a stacked on
- # b stacked on a.
- a = self.make_branch('a', format='1.6')
- b = self.make_branch('b', format='1.6')
- a.set_stacked_on_url(b.base)
- b.set_stacked_on_url(a.base)
- opener = self.makeBranchMirrorer([a.base, b.base])
- self.assertRaises(BranchLoopError, opener.open, a.base)
- self.assertRaises(BranchLoopError, opener.open, b.base)
-
-
-class TestReferenceMirroring(TestCaseWithTransport):
- """Feature tests for mirroring of branch references."""
+class TestReferenceOpener(TestCaseWithTransport):
+ """Feature tests for safe opening of branch references."""
def createBranchReference(self, url):
"""Create a pure branch reference that points to the specified URL.
@@ -513,19 +315,19 @@
self.assertEqual(opened_branch.base, target_branch.base)
def testFollowReferenceValue(self):
- # BranchMirrorer.followReference gives the reference value for
+ # SafeBranchOpener.followReference gives the reference value for
# a branch reference.
- opener = BranchMirrorer(BranchPolicy())
+ opener = SafeBranchOpener(BranchOpenPolicy())
reference_value = 'http://example.com/branch'
reference_url = self.createBranchReference(reference_value)
self.assertEqual(
reference_value, opener.followReference(reference_url))
def testFollowReferenceNone(self):
- # BranchMirrorer.followReference gives None for a normal branch.
+ # SafeBranchOpener.followReference gives None for a normal branch.
self.make_branch('repo')
branch_url = self.get_url('repo')
- opener = BranchMirrorer(BranchPolicy())
+ opener = SafeBranchOpener(BranchOpenPolicy())
self.assertIs(None, opener.followReference(branch_url))
=== modified file 'lib/lp/codehosting/puller/worker.py'
--- lib/lp/codehosting/puller/worker.py 2011-06-03 01:00:53 +0000
+++ lib/lp/codehosting/puller/worker.py 2011-08-06 17:48:46 +0000
@@ -8,20 +8,27 @@
import sys
import urllib2
-from bzrlib import errors
+from bzrlib import (
+ errors,
+ urlutils,
+ )
from bzrlib.branch import (
Branch,
BzrBranchFormat4,
)
-from bzrlib.bzrdir import BzrDir
from bzrlib.repofmt.weaverepo import (
RepositoryFormat4,
RepositoryFormat5,
RepositoryFormat6,
)
import bzrlib.ui
+from bzrlib.plugins.loom.branch import LoomSupport
+from bzrlib.transport import get_transport
from bzrlib.ui import SilentUIFactory
-from lazr.uri import InvalidURIError
+from lazr.uri import (
+ InvalidURIError,
+ URI,
+ )
from canonical.config import config
from canonical.launchpad.webapp import errorlog
@@ -32,18 +39,21 @@
from lp.code.enums import BranchType
from lp.codehosting.bzrutils import identical_formats
from lp.codehosting.puller import get_lock_id_for_branch_id
-from lp.codehosting.vfs.branchfs import (
- BadUrlLaunchpad,
- BadUrlScheme,
- BadUrlSsh,
- make_branch_mirrorer,
+from lp.codehosting.safe_open import (
+ BadUrl,
+ BranchLoopError,
+ BranchOpenPolicy,
+ BranchReferenceForbidden,
+ SafeBranchOpener,
)
__all__ = [
+ 'BadUrlLaunchpad',
+ 'BadUrlScheme',
+ 'BadUrlSsh',
'BranchMirrorer',
- 'BranchLoopError',
- 'BranchReferenceForbidden',
+ 'BranchMirrorerPolicy',
'get_canonical_url_for_branch_name',
'install_worker_ui_factory',
'PullerWorker',
@@ -51,19 +61,22 @@
]
-class BranchReferenceForbidden(Exception):
- """Trying to mirror a branch reference and the branch type does not allow
- references.
- """
-
-
-class BranchLoopError(Exception):
- """Encountered a branch cycle.
-
- A URL may point to a branch reference or it may point to a stacked branch.
- In either case, it's possible for there to be a cycle in these references,
- and this exception is raised when we detect such a cycle.
- """
+class BadUrlSsh(BadUrl):
+ """Tried to access a branch from sftp or bzr+ssh."""
+
+
+class BadUrlLaunchpad(BadUrl):
+ """Tried to access a branch from launchpad.net."""
+
+
+class BadUrlScheme(BadUrl):
+ """Found a URL with an untrusted scheme."""
+
+ def __init__(self, scheme, url):
+ BadUrl.__init__(self, scheme, url)
+ self.scheme = scheme
+
+
def get_canonical_url_for_branch_name(unique_name):
@@ -120,12 +133,40 @@
self.sendEvent('log', fmt % args)
+class BranchMirrorerPolicy(BranchOpenPolicy):
+ """The policy for what branches to open and how to stack them."""
+
+ def createDestinationBranch(self, source_branch, destination_url):
+ """Create a destination branch for 'source_branch'.
+
+ Creates a branch at 'destination_url' that is has the same format as
+ 'source_branch'. Any content already at 'destination_url' will be
+ deleted. Generally the new branch will have no revisions, but they
+ will be copied for import branches, because this can be done safely
+ and efficiently with a vfs-level copy (see `ImportedBranchPolicy`).
+
+ :param source_branch: The Bazaar branch that will be mirrored.
+ :param destination_url: The place to make the destination branch. This
+ URL must point to a writable location.
+ :return: The destination branch.
+ """
+ raise NotImplementedError
+
+ def getStackedOnURLForDestinationBranch(self, source_branch,
+ destination_url):
+ """Get the stacked on URL for `source_branch`.
+
+ In particular, the URL it should be stacked on when it is mirrored to
+ `destination_url`.
+ """
+ return None
+
+
class BranchMirrorer(object):
"""A `BranchMirrorer` safely makes mirrors of branches.
- A `BranchMirrorer` has a `BranchPolicy` to tell it which URLs are safe to
- accesss, whether or not to follow branch references and how to stack
- branches when they are mirrored.
+ A `BranchMirrorer` has a `BranchOpenPolicy` to tell it which URLs are safe
+ to accesss and whether or not to follow branch references.
The mirrorer knows how to follow branch references, create new mirrors,
update existing mirrors, determine stacked-on branches and the like.
@@ -136,92 +177,20 @@
def __init__(self, policy, protocol=None, log=None):
"""Construct a branch opener with 'policy'.
- :param policy: A `BranchPolicy` that tells us what URLs are valid and
- similar things.
+ :param policy: A `BranchOpenPolicy` that tells us what URLs are valid
+ and similar things.
:param log: A callable which can be called with a format string and
arguments to log messages in the scheduler, or None, in which case
log messages are discarded.
"""
- self._seen_urls = set()
self.policy = policy
self.protocol = protocol
+ self.opener = SafeBranchOpener(policy)
if log is not None:
self.log = log
else:
self.log = lambda *args: None
- def _runWithTransformFallbackLocationHookInstalled(
- self, callable, *args, **kw):
- Branch.hooks.install_named_hook(
- 'transform_fallback_location', self.transformFallbackLocationHook,
- 'BranchMirrorer.transformFallbackLocationHook')
- try:
- return callable(*args, **kw)
- finally:
- # XXX 2008-11-24 MichaelHudson, bug=301472: This is the hacky way
- # to remove a hook. The linked bug report asks for an API to do
- # it.
- Branch.hooks['transform_fallback_location'].remove(
- self.transformFallbackLocationHook)
- # We reset _seen_urls here to avoid multiple calls to open giving
- # spurious loop exceptions.
- self._seen_urls = set()
-
- def open(self, url):
- """Open the Bazaar branch at url, first checking for safety.
-
- What safety means is defined by a subclasses `followReference` and
- `checkOneURL` methods.
- """
- url = self.checkAndFollowBranchReference(url)
- return self._runWithTransformFallbackLocationHookInstalled(
- Branch.open, url)
-
- def transformFallbackLocationHook(self, branch, url):
- """Installed as the 'transform_fallback_location' Branch hook.
-
- This method calls `transformFallbackLocation` on the policy object and
- either returns the url it provides or passes it back to
- checkAndFollowBranchReference.
- """
- new_url, check = self.policy.transformFallbackLocation(branch, url)
- if check:
- return self.checkAndFollowBranchReference(new_url)
- else:
- return new_url
-
- def followReference(self, url):
- """Get the branch-reference value at the specified url.
-
- This exists as a separate method only to be overriden in unit tests.
- """
- bzrdir = BzrDir.open(url)
- return bzrdir.get_branch_reference()
-
- def checkAndFollowBranchReference(self, url):
- """Check URL (and possibly the referenced URL) for safety.
-
- This method checks that `url` passes the policy's `checkOneURL`
- method, and if `url` refers to a branch reference, it checks whether
- references are allowed and whether the reference's URL passes muster
- also -- recursively, until a real branch is found.
-
- :raise BranchLoopError: If the branch references form a loop.
- :raise BranchReferenceForbidden: If this opener forbids branch
- references.
- """
- while True:
- if url in self._seen_urls:
- raise BranchLoopError()
- self._seen_urls.add(url)
- self.policy.checkOneURL(url)
- next_url = self.followReference(url)
- if next_url is None:
- return url
- url = next_url
- if not self.policy.shouldFollowReferences():
- raise BranchReferenceForbidden(url)
-
def createDestinationBranch(self, source_branch, destination_url):
"""Create a destination branch for 'source_branch'.
@@ -234,7 +203,7 @@
URL must point to a writable location.
:return: The destination branch.
"""
- return self._runWithTransformFallbackLocationHookInstalled(
+ return self.opener.runWithTransformFallbackLocationHookInstalled(
self.policy.createDestinationBranch, source_branch,
destination_url)
@@ -296,6 +265,9 @@
stacked_on_url = self.updateBranch(source_branch, branch)
return branch, revid_before, stacked_on_url
+ def open(self, url):
+ return self.opener.open(url)
+
class PullerWorker:
"""This class represents a single branch that needs mirroring.
@@ -305,7 +277,7 @@
"""
def _checkerForBranchType(self, branch_type):
- """Return a `BranchMirrorer` with an appropriate `BranchPolicy`.
+ """Return a `BranchMirrorer` with an appropriate policy.
:param branch_type: A `BranchType`. The policy of the mirrorer will
be based on this.
@@ -546,3 +518,177 @@
created by another puller worker process.
"""
bzrlib.ui.ui_factory = PullerWorkerUIFactory(puller_worker_protocol)
+
+
+class MirroredBranchPolicy(BranchMirrorerPolicy):
+ """Mirroring policy for MIRRORED branches.
+
+ In summary:
+
+ - follow references,
+ - only open non-Launchpad http: and https: URLs.
+ """
+
+ def __init__(self, stacked_on_url=None):
+ self.stacked_on_url = stacked_on_url
+
+ def createDestinationBranch(self, source_branch, destination_url):
+ """Create a destination branch for 'source_branch'.
+
+ Creates a branch at 'destination_url' that is has the same format as
+ 'source_branch'. Any content already at 'destination_url' will be
+ deleted. Generally the new branch will have no revisions, but they
+ will be copied for import branches, because this can be done safely
+ and efficiently with a vfs-level copy (see `ImportedBranchPolicy`).
+
+ :param source_branch: The Bazaar branch that will be mirrored.
+ :param destination_url: The place to make the destination branch. This
+ URL must point to a writable location.
+ :return: The destination branch.
+ """
+ dest_transport = get_transport(destination_url)
+ if dest_transport.has('.'):
+ dest_transport.delete_tree('.')
+ if isinstance(source_branch, LoomSupport):
+ # Looms suck.
+ revision_id = None
+ else:
+ revision_id = 'null:'
+ source_branch.bzrdir.clone_on_transport(
+ dest_transport, revision_id=revision_id)
+ return Branch.open(destination_url)
+
+ def getStackedOnURLForDestinationBranch(self, source_branch,
+ destination_url):
+ """Return the stacked on URL for the destination branch.
+
+ Mirrored branches are stacked on the default stacked-on branch of
+ their product, except when we're mirroring the default stacked-on
+ branch itself.
+ """
+ if self.stacked_on_url is None:
+ return None
+ stacked_on_url = urlutils.join(destination_url, self.stacked_on_url)
+ if destination_url == stacked_on_url:
+ return None
+ return self.stacked_on_url
+
+ def shouldFollowReferences(self):
+ """See `BranchOpenPolicy.shouldFollowReferences`.
+
+ We traverse branch references for MIRRORED branches because they
+ provide a useful redirection mechanism and we want to be consistent
+ with the bzr command line.
+ """
+ return True
+
+ def transformFallbackLocation(self, branch, url):
+ """See `BranchOpenPolicy.transformFallbackLocation`.
+
+ For mirrored branches, we stack on whatever the remote branch claims
+ to stack on, but this URL still needs to be checked.
+ """
+ return urlutils.join(branch.base, url), True
+
+ def checkOneURL(self, url):
+ """See `BranchOpenPolicy.checkOneURL`.
+
+ We refuse to mirror from Launchpad or a ssh-like or file URL.
+ """
+ # Avoid circular import
+ from lp.code.interfaces.branch import get_blacklisted_hostnames
+ uri = URI(url)
+ launchpad_domain = config.vhost.mainsite.hostname
+ if uri.underDomain(launchpad_domain):
+ raise BadUrlLaunchpad(url)
+ for hostname in get_blacklisted_hostnames():
+ if uri.underDomain(hostname):
+ raise BadUrl(url)
+ if uri.scheme in ['sftp', 'bzr+ssh']:
+ raise BadUrlSsh(url)
+ elif uri.scheme not in ['http', 'https']:
+ raise BadUrlScheme(uri.scheme, url)
+
+
+class ImportedBranchPolicy(BranchMirrorerPolicy):
+ """Mirroring policy for IMPORTED branches.
+
+ In summary:
+
+ - don't follow references,
+ - assert the URLs start with the prefix we expect for imported branches.
+ """
+
+ def createDestinationBranch(self, source_branch, destination_url):
+ """See `BranchOpenPolicy.createDestinationBranch`.
+
+ Because we control the process that creates import branches, a
+ vfs-level copy is safe and more efficient than a bzr fetch.
+ """
+ source_transport = source_branch.bzrdir.root_transport
+ dest_transport = get_transport(destination_url)
+ while True:
+ # We loop until the remote file list before and after the copy is
+ # the same to catch the case where the remote side is being
+ # mutated as we copy it.
+ if dest_transport.has('.'):
+ dest_transport.delete_tree('.')
+ files_before = set(source_transport.iter_files_recursive())
+ source_transport.copy_tree_to_transport(dest_transport)
+ files_after = set(source_transport.iter_files_recursive())
+ if files_before == files_after:
+ break
+ return Branch.open_from_transport(dest_transport)
+
+ def shouldFollowReferences(self):
+ """See `BranchOpenerPolicy.shouldFollowReferences`.
+
+ We do not traverse references for IMPORTED branches because the
+ code-import system should never produce branch references.
+ """
+ return False
+
+ def transformFallbackLocation(self, branch, url):
+ """See `BranchOpenerPolicy.transformFallbackLocation`.
+
+ Import branches should not be stacked, ever.
+ """
+ raise AssertionError("Import branch unexpectedly stacked!")
+
+ def checkOneURL(self, url):
+ """See `BranchOpenerPolicy.checkOneURL`.
+
+ If the URL we are mirroring from does not start how we expect the pull
+ URLs of import branches to start, something has gone badly wrong, so
+ we raise AssertionError if that's happened.
+ """
+ if not url.startswith(config.launchpad.bzr_imports_root_url):
+ raise AssertionError(
+ "Bogus URL for imported branch: %r" % url)
+
+
+def make_branch_mirrorer(branch_type, protocol=None,
+ mirror_stacked_on_url=None):
+ """Create a `BranchMirrorer` with the appropriate `BranchOpenerPolicy`.
+
+ :param branch_type: A `BranchType` to select a policy by.
+ :param protocol: Optional protocol for the mirrorer to work with.
+ If given, its log will also be used.
+ :param mirror_stacked_on_url: For mirrored branches, the default URL
+ to stack on. Ignored for other branch types.
+ :return: A `BranchMirrorer`.
+ """
+ if branch_type == BranchType.MIRRORED:
+ policy = MirroredBranchPolicy(mirror_stacked_on_url)
+ elif branch_type == BranchType.IMPORTED:
+ policy = ImportedBranchPolicy()
+ else:
+ raise AssertionError(
+ "Unexpected branch type: %r" % branch_type)
+
+ if protocol is not None:
+ log_function = protocol.log
+ else:
+ log_function = None
+
+ return BranchMirrorer(policy, protocol, log_function)
=== added file 'lib/lp/codehosting/safe_open.py'
--- lib/lp/codehosting/safe_open.py 1970-01-01 00:00:00 +0000
+++ lib/lp/codehosting/safe_open.py 2011-08-06 17:48:46 +0000
@@ -0,0 +1,232 @@
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Safe branch opening."""
+
+__metaclass__ = type
+
+from bzrlib import urlutils
+from bzrlib.branch import Branch
+from bzrlib.bzrdir import BzrDir
+
+__all__ = [
+ 'AcceptAnythingPolicy',
+ 'BadUrl',
+ 'BlacklistPolicy',
+ 'BranchLoopError',
+ 'BranchReferenceForbidden',
+ 'SafeBranchOpener',
+ 'WhitelistPolicy',
+ ]
+
+
+# TODO JelmerVernooij 2011-08-06: This module is generic enough to be
+# in bzrlib, and may be of use to others.
+
+
+class BadUrl(Exception):
+ """Tried to access a branch from a bad URL."""
+
+
+class BranchReferenceForbidden(Exception):
+ """Trying to mirror a branch reference and the branch type does not allow
+ references.
+ """
+
+
+class BranchLoopError(Exception):
+ """Encountered a branch cycle.
+
+ A URL may point to a branch reference or it may point to a stacked branch.
+ In either case, it's possible for there to be a cycle in these references,
+ and this exception is raised when we detect such a cycle.
+ """
+
+
+class BranchOpenPolicy:
+ """Policy on how to open branches.
+
+ In particular, a policy determines which branches are safe to open by
+ checking their URLs and deciding whether or not to follow branch
+ references.
+ """
+
+ def shouldFollowReferences(self):
+ """Whether we traverse references when mirroring.
+
+ Subclasses must override this method.
+
+ If we encounter a branch reference and this returns false, an error is
+ raised.
+
+ :returns: A boolean to indicate whether to follow a branch reference.
+ """
+ raise NotImplementedError(self.shouldFollowReferences)
+
+ def transformFallbackLocation(self, branch, url):
+ """Validate, maybe modify, 'url' to be used as a stacked-on location.
+
+ :param branch: The branch that is being opened.
+ :param url: The URL that the branch provides for its stacked-on
+ location.
+ :return: (new_url, check) where 'new_url' is the URL of the branch to
+ actually open and 'check' is true if 'new_url' needs to be
+ validated by checkAndFollowBranchReference.
+ """
+ raise NotImplementedError(self.transformFallbackLocation)
+
+ def checkOneURL(self, url):
+ """Check the safety of the source URL.
+
+ Subclasses must override this method.
+
+ :param url: The source URL to check.
+ :raise BadUrl: subclasses are expected to raise this or a subclass
+ when it finds a URL it deems to be unsafe.
+ """
+ raise NotImplementedError(self.checkOneURL)
+
+
+class BlacklistPolicy(BranchOpenPolicy):
+ """Branch policy that forbids certain URLs."""
+
+ def __init__(self, should_follow_references, unsafe_urls=None):
+ if unsafe_urls is None:
+ unsafe_urls = set()
+ self._unsafe_urls = unsafe_urls
+ self._should_follow_references = should_follow_references
+
+ def shouldFollowReferences(self):
+ return self._should_follow_references
+
+ def checkOneURL(self, url):
+ if url in self._unsafe_urls:
+ raise BadUrl(url)
+
+ def transformFallbackLocation(self, branch, url):
+ """See `BranchOpenPolicy.transformFallbackLocation`.
+
+ This class is not used for testing our smarter stacking features so we
+ just do the simplest thing: return the URL that would be used anyway
+ and don't check it.
+ """
+ return urlutils.join(branch.base, url), False
+
+
+class AcceptAnythingPolicy(BlacklistPolicy):
+ """Accept anything, to make testing easier."""
+
+ def __init__(self):
+ super(AcceptAnythingPolicy, self).__init__(True, set())
+
+
+class WhitelistPolicy(BranchOpenPolicy):
+ """Branch policy that only allows certain URLs."""
+
+ def __init__(self, should_follow_references, allowed_urls=None,
+ check=False):
+ if allowed_urls is None:
+ allowed_urls = []
+ self.allowed_urls = set(url.rstrip('/') for url in allowed_urls)
+ self.check = check
+
+ def shouldFollowReferences(self):
+ return self._should_follow_references
+
+ def checkOneURL(self, url):
+ if url.rstrip('/') not in self.allowed_urls:
+ raise BadUrl(url)
+
+ def transformFallbackLocation(self, branch, url):
+ """See `BranchOpenPolicy.transformFallbackLocation`.
+
+ Here we return the URL that would be used anyway and optionally check
+ it.
+ """
+ return urlutils.join(branch.base, url), self.check
+
+
+class SafeBranchOpener(object):
+ """Safe branch opener.
+
+ The policy object is expected to have the following methods:
+ * checkOneURL
+ * shouldFollowReferences
+ * transformFallbackLocation
+ """
+
+ def __init__(self, policy):
+ self.policy = policy
+ self._seen_urls = set()
+
+ def checkAndFollowBranchReference(self, url):
+ """Check URL (and possibly the referenced URL) for safety.
+
+ This method checks that `url` passes the policy's `checkOneURL`
+ method, and if `url` refers to a branch reference, it checks whether
+ references are allowed and whether the reference's URL passes muster
+ also -- recursively, until a real branch is found.
+
+ :raise BranchLoopError: If the branch references form a loop.
+ :raise BranchReferenceForbidden: If this opener forbids branch
+ references.
+ """
+ while True:
+ if url in self._seen_urls:
+ raise BranchLoopError()
+ self._seen_urls.add(url)
+ self.policy.checkOneURL(url)
+ next_url = self.followReference(url)
+ if next_url is None:
+ return url
+ url = next_url
+ if not self.policy.shouldFollowReferences():
+ raise BranchReferenceForbidden(url)
+
+ def transformFallbackLocationHook(self, branch, url):
+ """Installed as the 'transform_fallback_location' Branch hook.
+
+ This method calls `transformFallbackLocation` on the policy object and
+ either returns the url it provides or passes it back to
+ checkAndFollowBranchReference.
+ """
+ new_url, check = self.policy.transformFallbackLocation(branch, url)
+ if check:
+ return self.checkAndFollowBranchReference(new_url)
+ else:
+ return new_url
+
+ def runWithTransformFallbackLocationHookInstalled(
+ self, callable, *args, **kw):
+ Branch.hooks.install_named_hook(
+ 'transform_fallback_location', self.transformFallbackLocationHook,
+ 'SafeBranchOpener.transformFallbackLocationHook')
+ try:
+ return callable(*args, **kw)
+ finally:
+ # XXX 2008-11-24 MichaelHudson, bug=301472: This is the hacky way
+ # to remove a hook. The linked bug report asks for an API to do
+ # it.
+ Branch.hooks['transform_fallback_location'].remove(
+ self.transformFallbackLocationHook)
+ # We reset _seen_urls here to avoid multiple calls to open giving
+ # spurious loop exceptions.
+ self._seen_urls = set()
+
+ def followReference(self, url):
+ """Get the branch-reference value at the specified url.
+
+ This exists as a separate method only to be overriden in unit tests.
+ """
+ bzrdir = BzrDir.open(url)
+ return bzrdir.get_branch_reference()
+
+ def open(self, url):
+ """Open the Bazaar branch at url, first checking for safety.
+
+ What safety means is defined by a subclasses `followReference` and
+ `checkOneURL` methods.
+ """
+ url = self.checkAndFollowBranchReference(url)
+ return self.runWithTransformFallbackLocationHookInstalled(
+ Branch.open, url)
=== added file 'lib/lp/codehosting/tests/test_safe_open.py'
--- lib/lp/codehosting/tests/test_safe_open.py 1970-01-01 00:00:00 +0000
+++ lib/lp/codehosting/tests/test_safe_open.py 2011-08-06 17:48:46 +0000
@@ -0,0 +1,223 @@
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the safe branch open code."""
+
+
+__metaclass__ = type
+
+
+from lp.codehosting.puller.worker import (
+ BranchLoopError,
+ BranchReferenceForbidden,
+ SafeBranchOpener,
+ )
+from lp.codehosting.safe_open import (
+ BadUrl,
+ BlacklistPolicy,
+ WhitelistPolicy,
+ )
+
+from lp.testing import TestCase
+
+from bzrlib.branch import (
+ BzrBranchFormat7,
+ )
+from bzrlib.bzrdir import (
+ BzrDirMetaFormat1,
+ )
+from bzrlib.repofmt.pack_repo import RepositoryFormatKnitPack1
+from bzrlib.tests import (
+ TestCaseWithTransport,
+ )
+
+
+class TestSafeBranchOpenerCheckAndFollowBranchReference(TestCase):
+ """Unit tests for `SafeBranchOpener.checkAndFollowBranchReference`."""
+
+ class StubbedSafeBranchOpener(SafeBranchOpener):
+ """SafeBranchOpener that provides canned answers.
+
+ We implement the methods we need to to be able to control all the
+ inputs to the `BranchMirrorer.checkSource` method, which is what is
+ being tested in this class.
+ """
+
+ def __init__(self, references, policy):
+ parent_cls = TestSafeBranchOpenerCheckAndFollowBranchReference
+ super(parent_cls.StubbedSafeBranchOpener, self).__init__(policy)
+ self._reference_values = {}
+ for i in range(len(references) - 1):
+ self._reference_values[references[i]] = references[i+1]
+ self.follow_reference_calls = []
+
+ def followReference(self, url):
+ self.follow_reference_calls.append(url)
+ return self._reference_values[url]
+
+ def makeBranchOpener(self, should_follow_references, references,
+ unsafe_urls=None):
+ policy = BlacklistPolicy(should_follow_references, unsafe_urls)
+ opener = self.StubbedSafeBranchOpener(references, policy)
+ return opener
+
+ def testCheckInitialURL(self):
+ # checkSource rejects all URLs that are not allowed.
+ opener = self.makeBranchOpener(None, [], set(['a']))
+ self.assertRaises(BadUrl, opener.checkAndFollowBranchReference, 'a')
+
+ def testNotReference(self):
+ # When branch references are forbidden, checkAndFollowBranchReference
+ # does not raise on non-references.
+ opener = self.makeBranchOpener(False, ['a', None])
+ self.assertEquals('a', opener.checkAndFollowBranchReference('a'))
+ self.assertEquals(['a'], opener.follow_reference_calls)
+
+ def testBranchReferenceForbidden(self):
+ # checkAndFollowBranchReference raises BranchReferenceForbidden if
+ # branch references are forbidden and the source URL points to a
+ # branch reference.
+ opener = self.makeBranchOpener(False, ['a', 'b'])
+ self.assertRaises(
+ BranchReferenceForbidden,
+ opener.checkAndFollowBranchReference, 'a')
+ self.assertEquals(['a'], opener.follow_reference_calls)
+
+ def testAllowedReference(self):
+ # checkAndFollowBranchReference does not raise if following references
+ # is allowed and the source URL points to a branch reference to a
+ # permitted location.
+ opener = self.makeBranchOpener(True, ['a', 'b', None])
+ self.assertEquals('b', opener.checkAndFollowBranchReference('a'))
+ self.assertEquals(['a', 'b'], opener.follow_reference_calls)
+
+ def testCheckReferencedURLs(self):
+ # checkAndFollowBranchReference checks if the URL a reference points
+ # to is safe.
+ opener = self.makeBranchOpener(
+ True, ['a', 'b', None], unsafe_urls=set('b'))
+ self.assertRaises(BadUrl, opener.checkAndFollowBranchReference, 'a')
+ self.assertEquals(['a'], opener.follow_reference_calls)
+
+ def testSelfReferencingBranch(self):
+ # checkAndFollowBranchReference raises BranchReferenceLoopError if
+ # following references is allowed and the source url points to a
+ # self-referencing branch reference.
+ opener = self.makeBranchOpener(True, ['a', 'a'])
+ self.assertRaises(
+ BranchLoopError, opener.checkAndFollowBranchReference, 'a')
+ self.assertEquals(['a'], opener.follow_reference_calls)
+
+ def testBranchReferenceLoop(self):
+ # checkAndFollowBranchReference raises BranchReferenceLoopError if
+ # following references is allowed and the source url points to a loop
+ # of branch references.
+ references = ['a', 'b', 'a']
+ opener = self.makeBranchOpener(True, references)
+ self.assertRaises(
+ BranchLoopError, opener.checkAndFollowBranchReference, 'a')
+ self.assertEquals(['a', 'b'], opener.follow_reference_calls)
+
+
+class TestSafeBranchOpenerStacking(TestCaseWithTransport):
+
+ def makeBranchOpener(self, allowed_urls):
+ policy = WhitelistPolicy(True, allowed_urls, True)
+ return SafeBranchOpener(policy)
+
+ def makeBranch(self, path, branch_format, repository_format):
+ """Make a Bazaar branch at 'path' with the given formats."""
+ bzrdir_format = BzrDirMetaFormat1()
+ bzrdir_format.set_branch_format(branch_format)
+ bzrdir = self.make_bzrdir(path, format=bzrdir_format)
+ repository_format.initialize(bzrdir)
+ return bzrdir.create_branch()
+
+ def testAllowedURL(self):
+ # checkSource does not raise an exception for branches stacked on
+ # branches with allowed URLs.
+ stacked_on_branch = self.make_branch('base-branch', format='1.6')
+ stacked_branch = self.make_branch('stacked-branch', format='1.6')
+ stacked_branch.set_stacked_on_url(stacked_on_branch.base)
+ opener = self.makeBranchOpener(
+ [stacked_branch.base, stacked_on_branch.base])
+ # This doesn't raise an exception.
+ opener.open(stacked_branch.base)
+
+ def testUnstackableRepository(self):
+ # checkSource treats branches with UnstackableRepositoryFormats as
+ # being not stacked.
+ branch = self.makeBranch(
+ 'unstacked', BzrBranchFormat7(), RepositoryFormatKnitPack1())
+ opener = self.makeBranchOpener([branch.base])
+ # This doesn't raise an exception.
+ opener.open(branch.base)
+
+ def testAllowedRelativeURL(self):
+ # checkSource passes on absolute urls to checkOneURL, even if the
+ # value of stacked_on_location in the config is set to a relative URL.
+ stacked_on_branch = self.make_branch('base-branch', format='1.6')
+ stacked_branch = self.make_branch('stacked-branch', format='1.6')
+ stacked_branch.set_stacked_on_url('../base-branch')
+ opener = self.makeBranchOpener(
+ [stacked_branch.base, stacked_on_branch.base])
+ # Note that stacked_on_branch.base is not '../base-branch', it's an
+ # absolute URL.
+ self.assertNotEqual('../base-branch', stacked_on_branch.base)
+ # This doesn't raise an exception.
+ opener.open(stacked_branch.base)
+
+ def testAllowedRelativeNested(self):
+ # Relative URLs are resolved relative to the stacked branch.
+ self.get_transport().mkdir('subdir')
+ a = self.make_branch('subdir/a', format='1.6')
+ b = self.make_branch('b', format='1.6')
+ b.set_stacked_on_url('../subdir/a')
+ c = self.make_branch('subdir/c', format='1.6')
+ c.set_stacked_on_url('../../b')
+ opener = self.makeBranchOpener([c.base, b.base, a.base])
+ # This doesn't raise an exception.
+ opener.open(c.base)
+
+ def testForbiddenURL(self):
+ # checkSource raises a BadUrl exception if a branch is stacked on a
+ # branch with a forbidden URL.
+ stacked_on_branch = self.make_branch('base-branch', format='1.6')
+ stacked_branch = self.make_branch('stacked-branch', format='1.6')
+ stacked_branch.set_stacked_on_url(stacked_on_branch.base)
+ opener = self.makeBranchOpener([stacked_branch.base])
+ self.assertRaises(BadUrl, opener.open, stacked_branch.base)
+
+ def testForbiddenURLNested(self):
+ # checkSource raises a BadUrl exception if a branch is stacked on a
+ # branch that is in turn stacked on a branch with a forbidden URL.
+ a = self.make_branch('a', format='1.6')
+ b = self.make_branch('b', format='1.6')
+ b.set_stacked_on_url(a.base)
+ c = self.make_branch('c', format='1.6')
+ c.set_stacked_on_url(b.base)
+ opener = self.makeBranchOpener([c.base, b.base])
+ self.assertRaises(BadUrl, opener.open, c.base)
+
+ def testSelfStackedBranch(self):
+ # checkSource raises StackingLoopError if a branch is stacked on
+ # itself. This avoids infinite recursion errors.
+ a = self.make_branch('a', format='1.6')
+ # Bazaar 1.17 and up make it harder to create branches like this.
+ # It's still worth testing that we don't blow up in the face of them,
+ # so we grovel around a bit to create one anyway.
+ a.get_config().set_user_option('stacked_on_location', a.base)
+ opener = self.makeBranchOpener([a.base])
+ self.assertRaises(BranchLoopError, opener.open, a.base)
+
+ def testLoopStackedBranch(self):
+ # checkSource raises StackingLoopError if a branch is stacked in such
+ # a way so that it is ultimately stacked on itself. e.g. a stacked on
+ # b stacked on a.
+ a = self.make_branch('a', format='1.6')
+ b = self.make_branch('b', format='1.6')
+ a.set_stacked_on_url(b.base)
+ b.set_stacked_on_url(a.base)
+ opener = self.makeBranchOpener([a.base, b.base])
+ self.assertRaises(BranchLoopError, opener.open, a.base)
+ self.assertRaises(BranchLoopError, opener.open, b.base)
=== modified file 'lib/lp/codehosting/vfs/__init__.py'
--- lib/lp/codehosting/vfs/__init__.py 2010-09-22 17:16:19 +0000
+++ lib/lp/codehosting/vfs/__init__.py 2011-08-06 17:48:46 +0000
@@ -11,7 +11,6 @@
'get_ro_server',
'get_rw_server',
'LaunchpadServer',
- 'make_branch_mirrorer',
]
from lp.codehosting.vfs.branchfs import (
@@ -21,7 +20,6 @@
get_ro_server,
get_rw_server,
LaunchpadServer,
- make_branch_mirrorer,
)
from lp.codehosting.vfs.branchfsclient import (
BranchFileSystemClient,
=== modified file 'lib/lp/codehosting/vfs/branchfs.py'
--- lib/lp/codehosting/vfs/branchfs.py 2011-05-25 19:57:07 +0000
+++ lib/lp/codehosting/vfs/branchfs.py 2011-08-06 17:48:46 +0000
@@ -46,17 +46,14 @@
__metaclass__ = type
__all__ = [
'AsyncLaunchpadTransport',
- 'BadUrl',
'BadUrlLaunchpad',
'BadUrlScheme',
'BadUrlSsh',
'branch_id_to_path',
- 'BranchPolicy',
'DirectDatabaseLaunchpadServer',
'get_lp_server',
'get_ro_server',
'get_rw_server',
- 'make_branch_mirrorer',
'LaunchpadInternalServer',
'LaunchpadServer',
]
@@ -65,7 +62,6 @@
import xmlrpclib
from bzrlib import urlutils
-from bzrlib.branch import Branch
from bzrlib.bzrdir import (
BzrDir,
BzrDirFormat,
@@ -76,7 +72,6 @@
PermissionDenied,
TransportNotPossible,
)
-from bzrlib.plugins.loom.branch import LoomSupport
from bzrlib.smart.request import jail_info
from bzrlib.transport import get_transport
from bzrlib.transport.memory import MemoryServer
@@ -98,7 +93,6 @@
from canonical.config import config
from canonical.launchpad.webapp import errorlog
from canonical.launchpad.xmlrpc import faults
-from lp.code.enums import BranchType
from lp.code.interfaces.branchlookup import IBranchLookup
from lp.code.interfaces.codehosting import (
BRANCH_TRANSPORT,
@@ -126,26 +120,6 @@
)
-class BadUrl(Exception):
- """Tried to access a branch from a bad URL."""
-
-
-class BadUrlSsh(BadUrl):
- """Tried to access a branch from sftp or bzr+ssh."""
-
-
-class BadUrlLaunchpad(BadUrl):
- """Tried to access a branch from launchpad.net."""
-
-
-class BadUrlScheme(BadUrl):
- """Found a URL with an untrusted scheme."""
-
- def __init__(self, scheme, url):
- BadUrl.__init__(self, scheme, url)
- self.scheme = scheme
-
-
# The directories allowed directly beneath a branch directory. These are the
# directories that Bazaar creates as part of regular operation. We support
# only two numbered backups to avoid indefinite space usage.
@@ -774,234 +748,3 @@
seen_new_branch_hook)
return lp_server
-
-class BranchPolicy:
- """Policy on how to mirror branches.
-
- In particular, a policy determines which branches are safe to mirror by
- checking their URLs and deciding whether or not to follow branch
- references. A policy also determines how the mirrors of branches should be
- stacked.
- """
-
- def createDestinationBranch(self, source_branch, destination_url):
- """Create a destination branch for 'source_branch'.
-
- Creates a branch at 'destination_url' that is has the same format as
- 'source_branch'. Any content already at 'destination_url' will be
- deleted. Generally the new branch will have no revisions, but they
- will be copied for import branches, because this can be done safely
- and efficiently with a vfs-level copy (see `ImportedBranchPolicy`,
- below).
-
- :param source_branch: The Bazaar branch that will be mirrored.
- :param destination_url: The place to make the destination branch. This
- URL must point to a writable location.
- :return: The destination branch.
- """
- dest_transport = get_transport(destination_url)
- if dest_transport.has('.'):
- dest_transport.delete_tree('.')
- if isinstance(source_branch, LoomSupport):
- # Looms suck.
- revision_id = None
- else:
- revision_id = 'null:'
- source_branch.bzrdir.clone_on_transport(
- dest_transport, revision_id=revision_id)
- return Branch.open(destination_url)
-
- def getStackedOnURLForDestinationBranch(self, source_branch,
- destination_url):
- """Get the stacked on URL for `source_branch`.
-
- In particular, the URL it should be stacked on when it is mirrored to
- `destination_url`.
- """
- return None
-
- def shouldFollowReferences(self):
- """Whether we traverse references when mirroring.
-
- Subclasses must override this method.
-
- If we encounter a branch reference and this returns false, an error is
- raised.
-
- :returns: A boolean to indicate whether to follow a branch reference.
- """
- raise NotImplementedError(self.shouldFollowReferences)
-
- def transformFallbackLocation(self, branch, url):
- """Validate, maybe modify, 'url' to be used as a stacked-on location.
-
- :param branch: The branch that is being opened.
- :param url: The URL that the branch provides for its stacked-on
- location.
- :return: (new_url, check) where 'new_url' is the URL of the branch to
- actually open and 'check' is true if 'new_url' needs to be
- validated by checkAndFollowBranchReference.
- """
- raise NotImplementedError(self.transformFallbackLocation)
-
- def checkOneURL(self, url):
- """Check the safety of the source URL.
-
- Subclasses must override this method.
-
- :param url: The source URL to check.
- :raise BadUrl: subclasses are expected to raise this or a subclass
- when it finds a URL it deems to be unsafe.
- """
- raise NotImplementedError(self.checkOneURL)
-
-
-class MirroredBranchPolicy(BranchPolicy):
- """Mirroring policy for MIRRORED branches.
-
- In summary:
-
- - follow references,
- - only open non-Launchpad http: and https: URLs.
- """
-
- def __init__(self, stacked_on_url=None):
- self.stacked_on_url = stacked_on_url
-
- def getStackedOnURLForDestinationBranch(self, source_branch,
- destination_url):
- """See `BranchPolicy.getStackedOnURLForDestinationBranch`.
-
- Mirrored branches are stacked on the default stacked-on branch of
- their product, except when we're mirroring the default stacked-on
- branch itself.
- """
- if self.stacked_on_url is None:
- return None
- stacked_on_url = urlutils.join(destination_url, self.stacked_on_url)
- if destination_url == stacked_on_url:
- return None
- return self.stacked_on_url
-
- def shouldFollowReferences(self):
- """See `BranchPolicy.shouldFollowReferences`.
-
- We traverse branch references for MIRRORED branches because they
- provide a useful redirection mechanism and we want to be consistent
- with the bzr command line.
- """
- return True
-
- def transformFallbackLocation(self, branch, url):
- """See `BranchPolicy.transformFallbackLocation`.
-
- For mirrored branches, we stack on whatever the remote branch claims
- to stack on, but this URL still needs to be checked.
- """
- return urlutils.join(branch.base, url), True
-
- def checkOneURL(self, url):
- """See `BranchPolicy.checkOneURL`.
-
- We refuse to mirror from Launchpad or a ssh-like or file URL.
- """
- # Avoid circular import
- from lp.code.interfaces.branch import get_blacklisted_hostnames
- uri = URI(url)
- launchpad_domain = config.vhost.mainsite.hostname
- if uri.underDomain(launchpad_domain):
- raise BadUrlLaunchpad(url)
- for hostname in get_blacklisted_hostnames():
- if uri.underDomain(hostname):
- raise BadUrl(url)
- if uri.scheme in ['sftp', 'bzr+ssh']:
- raise BadUrlSsh(url)
- elif uri.scheme not in ['http', 'https']:
- raise BadUrlScheme(uri.scheme, url)
-
-
-class ImportedBranchPolicy(BranchPolicy):
- """Mirroring policy for IMPORTED branches.
-
- In summary:
-
- - don't follow references,
- - assert the URLs start with the prefix we expect for imported branches.
- """
-
- def createDestinationBranch(self, source_branch, destination_url):
- """See `BranchPolicy.createDestinationBranch`.
-
- Because we control the process that creates import branches, a
- vfs-level copy is safe and more efficient than a bzr fetch.
- """
- source_transport = source_branch.bzrdir.root_transport
- dest_transport = get_transport(destination_url)
- while True:
- # We loop until the remote file list before and after the copy is
- # the same to catch the case where the remote side is being
- # mutated as we copy it.
- if dest_transport.has('.'):
- dest_transport.delete_tree('.')
- files_before = set(source_transport.iter_files_recursive())
- source_transport.copy_tree_to_transport(dest_transport)
- files_after = set(source_transport.iter_files_recursive())
- if files_before == files_after:
- break
- return Branch.open_from_transport(dest_transport)
-
- def shouldFollowReferences(self):
- """See `BranchPolicy.shouldFollowReferences`.
-
- We do not traverse references for IMPORTED branches because the
- code-import system should never produce branch references.
- """
- return False
-
- def transformFallbackLocation(self, branch, url):
- """See `BranchPolicy.transformFallbackLocation`.
-
- Import branches should not be stacked, ever.
- """
- raise AssertionError("Import branch unexpectedly stacked!")
-
- def checkOneURL(self, url):
- """See `BranchPolicy.checkOneURL`.
-
- If the URL we are mirroring from does not start how we expect the pull
- URLs of import branches to start, something has gone badly wrong, so
- we raise AssertionError if that's happened.
- """
- if not url.startswith(config.launchpad.bzr_imports_root_url):
- raise AssertionError(
- "Bogus URL for imported branch: %r" % url)
-
-
-def make_branch_mirrorer(branch_type, protocol=None,
- mirror_stacked_on_url=None):
- """Create a `BranchMirrorer` with the appropriate `BranchPolicy`.
-
- :param branch_type: A `BranchType` to select a policy by.
- :param protocol: Optional protocol for the mirrorer to work with.
- If given, its log will also be used.
- :param mirror_stacked_on_url: For mirrored branches, the default URL
- to stack on. Ignored for other branch types.
- :return: A `BranchMirrorer`.
- """
- # Avoid circular import
- from lp.codehosting.puller.worker import BranchMirrorer
-
- if branch_type == BranchType.MIRRORED:
- policy = MirroredBranchPolicy(mirror_stacked_on_url)
- elif branch_type == BranchType.IMPORTED:
- policy = ImportedBranchPolicy()
- else:
- raise AssertionError(
- "Unexpected branch type: %r" % branch_type)
-
- if protocol is not None:
- log_function = protocol.log
- else:
- log_function = None
-
- return BranchMirrorer(policy, protocol, log_function)
