Merge lp:~jelmer/launchpad/use-bzr-policy-open into lp:launchpad
- use-bzr-policy-open
- Merge into devel
Proposed by
Jelmer Vernooij
Status: | Rejected |
---|---|
Rejected by: | Jelmer Vernooij |
Proposed branch: | lp:~jelmer/launchpad/use-bzr-policy-open |
Merge into: | lp:launchpad |
Prerequisite: | lp:~jelmer/launchpad/bzr-2.5.1 |
Diff against target: |
1424 lines (+95/-830) 16 files modified
lib/launchpad_loggerhead/app.py (+2/-2) lib/lp/code/bzr.py (+0/-1) lib/lp/code/interfaces/branch.py (+1/-1) lib/lp/code/model/branch.py (+2/-2) lib/lp/code/model/tests/test_branch.py (+3/-3) lib/lp/codehosting/codeimport/tests/test_worker.py (+17/-17) lib/lp/codehosting/codeimport/worker.py (+13/-12) lib/lp/codehosting/puller/tests/__init__.py (+1/-1) lib/lp/codehosting/puller/tests/test_errors.py (+4/-4) lib/lp/codehosting/puller/tests/test_worker.py (+24/-24) lib/lp/codehosting/puller/tests/test_worker_formats.py (+2/-2) lib/lp/codehosting/puller/worker.py (+23/-22) lib/lp/codehosting/safe_open.py (+0/-354) lib/lp/codehosting/scanner/tests/test_bzrsync.py (+2/-2) lib/lp/codehosting/tests/test_safe_open.py (+0/-382) scripts/code-import-worker.py (+1/-1) |
To merge this branch: | bzr merge lp:~jelmer/launchpad/use-bzr-policy-open |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
j.c.sackett (community) | Approve | ||
Review via email: mp+112529@code.launchpad.net |
Commit message
Use new bzrlib.
Description of the change
Launchpad's safe_open API was fairly generic and has been moved into bzrlib as bzrlib.
This branch changes Launchpad to use the copy of safe_open that is present in bzrlib, and removes it from Launchpad itself.
To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote : | # |
> One thing: On line 240 of your diff, you have 'svn' listed twice in the list
> of
> allowed_schemes. I assume that's just a typo/editing artifact.
Whoops, good point. Fixed.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/launchpad_loggerhead/app.py' |
2 | --- lib/launchpad_loggerhead/app.py 2012-08-15 20:57:12 +0000 |
3 | +++ lib/launchpad_loggerhead/app.py 2012-09-06 09:05:37 +0000 |
4 | @@ -19,6 +19,7 @@ |
5 | favicon_app, |
6 | static_app, |
7 | ) |
8 | +from bzrlib.url_policy_open import open_only_scheme |
9 | from loggerhead.apps.branch import BranchWSGIApp |
10 | import oops_wsgi |
11 | from openid.consumer.consumer import ( |
12 | @@ -47,7 +48,6 @@ |
13 | BRANCH_TRANSPORT, |
14 | LAUNCHPAD_ANONYMOUS, |
15 | ) |
16 | -from lp.codehosting.safe_open import safe_open |
17 | from lp.codehosting.vfs import get_lp_server |
18 | from lp.services.config import config |
19 | from lp.services.webapp.errorlog import ErrorReportingUtility |
20 | @@ -273,7 +273,7 @@ |
21 | response.close() |
22 | |
23 | try: |
24 | - bzr_branch = safe_open( |
25 | + bzr_branch = open_only_scheme( |
26 | lp_server.get_url().strip(':/'), branch_url) |
27 | except errors.NotBranchError as err: |
28 | self.log.warning('Not a branch: %s', err) |
29 | |
30 | === modified file 'lib/lp/code/bzr.py' |
31 | --- lib/lp/code/bzr.py 2012-08-30 14:52:37 +0000 |
32 | +++ lib/lp/code/bzr.py 2012-09-06 09:05:37 +0000 |
33 | @@ -71,7 +71,6 @@ |
34 | RepositoryFormatKnit3, |
35 | RepositoryFormatKnit4, |
36 | ) |
37 | -from bzrlib.tsort import topo_sort |
38 | from lazr.enum import ( |
39 | DBEnumeratedType, |
40 | DBItem, |
41 | |
42 | === modified file 'lib/lp/code/interfaces/branch.py' |
43 | --- lib/lp/code/interfaces/branch.py 2012-08-29 01:06:36 +0000 |
44 | +++ lib/lp/code/interfaces/branch.py 2012-09-06 09:05:37 +0000 |
45 | @@ -934,7 +934,7 @@ |
46 | You can only call this if a server returned by `get_ro_server` or |
47 | `get_rw_server` is running. |
48 | |
49 | - :raise lp.codehosting.safe_open.BadUrl: If the branch is stacked |
50 | + :raise bzrlib.url_policy_open.BadUrl: If the branch is stacked |
51 | on or a reference to an unacceptable URL. |
52 | """ |
53 | |
54 | |
55 | === modified file 'lib/lp/code/model/branch.py' |
56 | --- lib/lp/code/model/branch.py 2012-09-05 21:51:06 +0000 |
57 | +++ lib/lp/code/model/branch.py 2012-09-06 09:05:37 +0000 |
58 | @@ -15,6 +15,7 @@ |
59 | |
60 | from bzrlib import urlutils |
61 | from bzrlib.revision import NULL_REVISION |
62 | +from bzrlib.url_policy_open import open_only_scheme |
63 | import pytz |
64 | import simplejson |
65 | from sqlobject import ( |
66 | @@ -130,7 +131,6 @@ |
67 | RevisionAuthor, |
68 | ) |
69 | from lp.code.model.seriessourcepackagebranch import SeriesSourcePackageBranch |
70 | -from lp.codehosting.safe_open import safe_open |
71 | from lp.registry.enums import ( |
72 | InformationType, |
73 | PersonVisibility, |
74 | @@ -655,7 +655,7 @@ |
75 | |
76 | def getBzrBranch(self): |
77 | """See `IBranch`.""" |
78 | - return safe_open('lp-internal', self.getInternalBzrUrl()) |
79 | + return open_only_scheme('lp-internal', self.getInternalBzrUrl()) |
80 | |
81 | @property |
82 | def displayname(self): |
83 | |
84 | === modified file 'lib/lp/code/model/tests/test_branch.py' |
85 | --- lib/lp/code/model/tests/test_branch.py 2012-08-30 21:59:39 +0000 |
86 | +++ lib/lp/code/model/tests/test_branch.py 2012-09-06 09:05:37 +0000 |
87 | @@ -16,6 +16,7 @@ |
88 | from bzrlib.branch import Branch |
89 | from bzrlib.bzrdir import BzrDir |
90 | from bzrlib.revision import NULL_REVISION |
91 | +from bzrlib.url_policy_open import BadUrl |
92 | from pytz import UTC |
93 | import simplejson |
94 | from sqlobject import SQLObjectNotFound |
95 | @@ -109,7 +110,6 @@ |
96 | from lp.code.model.codereviewcomment import CodeReviewComment |
97 | from lp.code.model.revision import Revision |
98 | from lp.code.tests.helpers import add_revision_to_branch |
99 | -from lp.codehosting.safe_open import BadUrl |
100 | from lp.codehosting.vfs.branchfs import get_real_branch_path |
101 | from lp.registry.enums import ( |
102 | BranchSharingPolicy, |
103 | @@ -3213,7 +3213,7 @@ |
104 | self.useBzrBranches(direct_database=True) |
105 | |
106 | def test_simple(self): |
107 | - # safe_open returns the underlying bzr branch of a database branch in |
108 | + # open_only_scheme returns the underlying bzr branch of a database branch in |
109 | # the simple, unstacked, case. |
110 | db_branch, tree = self.create_branch_and_tree() |
111 | # XXX: AaronBentley 2010-08-06 bug=614404: a bzr username is |
112 | @@ -3225,7 +3225,7 @@ |
113 | |
114 | def test_acceptable_stacking(self): |
115 | # If the underlying bzr branch of a database branch is stacked on |
116 | - # another launchpad branch safe_open returns it. |
117 | + # another launchpad branch open_only_scheme returns it. |
118 | db_stacked_on, stacked_on_tree = self.create_branch_and_tree() |
119 | db_stacked, stacked_tree = self.create_branch_and_tree() |
120 | stacked_tree.branch.set_stacked_on_url( |
121 | |
122 | === modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py' |
123 | --- lib/lp/codehosting/codeimport/tests/test_worker.py 2012-06-26 08:19:13 +0000 |
124 | +++ lib/lp/codehosting/codeimport/tests/test_worker.py 2012-09-06 09:05:37 +0000 |
125 | @@ -39,6 +39,13 @@ |
126 | join as urljoin, |
127 | local_path_from_url, |
128 | ) |
129 | +from bzrlib.url_policy_open import ( |
130 | + _BlacklistPolicy, |
131 | + AcceptAnythingPolicy, |
132 | + BadUrl, |
133 | + BranchOpenPolicy, |
134 | + BranchOpener, |
135 | + ) |
136 | from CVS import ( |
137 | Repository, |
138 | tree as CVSTree, |
139 | @@ -79,13 +86,6 @@ |
140 | ImportDataStore, |
141 | ImportWorker, |
142 | ) |
143 | -from lp.codehosting.safe_open import ( |
144 | - AcceptAnythingPolicy, |
145 | - BadUrl, |
146 | - BlacklistPolicy, |
147 | - BranchOpenPolicy, |
148 | - SafeBranchOpener, |
149 | - ) |
150 | from lp.codehosting.tests.helpers import create_branch_with_one_revision |
151 | from lp.registry.enums import InformationType |
152 | from lp.services.config import config |
153 | @@ -139,7 +139,7 @@ |
154 | def setUp(self): |
155 | super(WorkerTest, self).setUp() |
156 | self.disable_directory_isolation() |
157 | - SafeBranchOpener.install_hook() |
158 | + BranchOpener.install_hook() |
159 | |
160 | def assertDirectoryTreesEqual(self, directory1, directory2): |
161 | """Assert that `directory1` has the same structure as `directory2`. |
162 | @@ -1359,7 +1359,7 @@ |
163 | reference_url, target_url = self.createBranchReference() |
164 | source_details = self.factory.makeCodeImportSourceDetails( |
165 | url=reference_url, rcstype='bzr') |
166 | - policy = BlacklistPolicy(True, [target_url]) |
167 | + policy = _BlacklistPolicy(True, [target_url]) |
168 | worker = self.makeImportWorker(source_details, opener_policy=policy) |
169 | self.assertEqual( |
170 | CodeImportWorkerExitCode.FAILURE_FORBIDDEN, worker.run()) |
171 | @@ -1391,15 +1391,15 @@ |
172 | self.policy = CodeImportBranchOpenPolicy() |
173 | |
174 | def test_follows_references(self): |
175 | - self.assertEquals(True, self.policy.shouldFollowReferences()) |
176 | + self.assertEquals(True, self.policy.should_follow_references()) |
177 | |
178 | def assertBadUrl(self, url): |
179 | - self.assertRaises(BadUrl, self.policy.checkOneURL, url) |
180 | + self.assertRaises(BadUrl, self.policy.check_one_url, url) |
181 | |
182 | def assertGoodUrl(self, url): |
183 | - self.policy.checkOneURL(url) |
184 | + self.policy.check_one_url(url) |
185 | |
186 | - def test_checkOneURL(self): |
187 | + def test_check_one_url(self): |
188 | self.assertBadUrl("sftp://somehost/") |
189 | self.assertBadUrl("/etc/passwd") |
190 | self.assertBadUrl("file:///etc/passwd") |
191 | @@ -1421,7 +1421,7 @@ |
192 | def setUp(self): |
193 | http_utils.TestCaseWithRedirectedWebserver.setUp(self) |
194 | self.disable_directory_isolation() |
195 | - SafeBranchOpener.install_hook() |
196 | + BranchOpener.install_hook() |
197 | tree = self.make_branch_and_tree('.') |
198 | self.revid = tree.commit("A commit") |
199 | self.bazaar_store = BazaarBranchStore( |
200 | @@ -1451,14 +1451,14 @@ |
201 | def __init__(self, new_url): |
202 | self.new_url = new_url |
203 | |
204 | - def shouldFollowReferences(self): |
205 | + def should_follow_references(self): |
206 | return True |
207 | |
208 | - def checkOneURL(self, url): |
209 | + def check_one_url(self, url): |
210 | if url.startswith(self.new_url): |
211 | raise BadUrl(url) |
212 | |
213 | - def transformFallbackLocation(self, branch, url): |
214 | + def transform_fallback_location(self, branch, url): |
215 | return urlutils.join(branch.base, url), False |
216 | |
217 | policy = NewUrlBlacklistPolicy(self.get_new_url()) |
218 | |
219 | === modified file 'lib/lp/codehosting/codeimport/worker.py' |
220 | --- lib/lp/codehosting/codeimport/worker.py 2012-06-29 08:40:05 +0000 |
221 | +++ lib/lp/codehosting/codeimport/worker.py 2012-09-06 09:05:37 +0000 |
222 | @@ -53,6 +53,12 @@ |
223 | join as urljoin, |
224 | local_path_from_url, |
225 | ) |
226 | +from bzrlib.url_policy_open import ( |
227 | + BadUrl, |
228 | + BranchOpenPolicy, |
229 | + BranchOpener, |
230 | + ) |
231 | + |
232 | import cscvs |
233 | from cscvs.cmds import totla |
234 | import CVS |
235 | @@ -77,11 +83,6 @@ |
236 | extract_tarball, |
237 | ) |
238 | from lp.codehosting.codeimport.uifactory import LoggingUIFactory |
239 | -from lp.codehosting.safe_open import ( |
240 | - BadUrl, |
241 | - BranchOpenPolicy, |
242 | - SafeBranchOpener, |
243 | - ) |
244 | from lp.services.config import config |
245 | from lp.services.propertycache import cachedproperty |
246 | |
247 | @@ -97,8 +98,8 @@ |
248 | |
249 | allowed_schemes = ['http', 'https', 'svn', 'git', 'ftp', 'bzr'] |
250 | |
251 | - def shouldFollowReferences(self): |
252 | - """See `BranchOpenPolicy.shouldFollowReferences`. |
253 | + def should_follow_references(self): |
254 | + """See `BranchOpenPolicy.should_follow_references`. |
255 | |
256 | We traverse branch references for MIRRORED branches because they |
257 | provide a useful redirection mechanism and we want to be consistent |
258 | @@ -106,16 +107,16 @@ |
259 | """ |
260 | return True |
261 | |
262 | - def transformFallbackLocation(self, branch, url): |
263 | - """See `BranchOpenPolicy.transformFallbackLocation`. |
264 | + def transform_fallback_location(self, branch, url): |
265 | + """See `BranchOpenPolicy.transform_fallback_location`. |
266 | |
267 | For mirrored branches, we stack on whatever the remote branch claims |
268 | to stack on, but this URL still needs to be checked. |
269 | """ |
270 | return urljoin(branch.base, url), True |
271 | |
272 | - def checkOneURL(self, url): |
273 | - """See `BranchOpenPolicy.checkOneURL`. |
274 | + def check_one_url(self, url): |
275 | + """See `BranchOpenPolicy.check_one_url`. |
276 | |
277 | We refuse to mirror from Launchpad or a ssh-like or file URL. |
278 | """ |
279 | @@ -717,7 +718,7 @@ |
280 | def _doImport(self): |
281 | self._logger.info("Starting job.") |
282 | saved_factory = bzrlib.ui.ui_factory |
283 | - opener = SafeBranchOpener(self._opener_policy, self.probers) |
284 | + opener = BranchOpener(self._opener_policy, self.probers) |
285 | bzrlib.ui.ui_factory = LoggingUIFactory(logger=self._logger) |
286 | try: |
287 | self._logger.info( |
288 | |
289 | === modified file 'lib/lp/codehosting/puller/tests/__init__.py' |
290 | --- lib/lp/codehosting/puller/tests/__init__.py 2012-04-16 23:02:44 +0000 |
291 | +++ lib/lp/codehosting/puller/tests/__init__.py 2012-09-06 09:05:37 +0000 |
292 | @@ -16,6 +16,7 @@ |
293 | TestingHTTPServer, |
294 | TestingThreadingHTTPServer, |
295 | ) |
296 | +from bzrlib.url_policy_open import AcceptAnythingPolicy |
297 | |
298 | from lp.codehosting.puller.worker import ( |
299 | BranchMirrorer, |
300 | @@ -23,7 +24,6 @@ |
301 | PullerWorker, |
302 | PullerWorkerProtocol, |
303 | ) |
304 | -from lp.codehosting.safe_open import AcceptAnythingPolicy |
305 | from lp.codehosting.tests.helpers import LoomTestMixin |
306 | from lp.testing import TestCaseWithFactory |
307 | |
308 | |
309 | === modified file 'lib/lp/codehosting/puller/tests/test_errors.py' |
310 | --- lib/lp/codehosting/puller/tests/test_errors.py 2011-11-14 08:30:52 +0000 |
311 | +++ lib/lp/codehosting/puller/tests/test_errors.py 2012-09-06 09:05:37 +0000 |
312 | @@ -18,6 +18,10 @@ |
313 | UnknownFormatError, |
314 | UnsupportedFormatError, |
315 | ) |
316 | +from bzrlib.url_policy_open import ( |
317 | + BranchLoopError, |
318 | + BranchReferenceForbidden, |
319 | + ) |
320 | from lazr.uri import InvalidURIError |
321 | |
322 | from lp.code.enums import BranchType |
323 | @@ -29,10 +33,6 @@ |
324 | PullerWorker, |
325 | PullerWorkerProtocol, |
326 | ) |
327 | -from lp.codehosting.safe_open import ( |
328 | - BranchLoopError, |
329 | - BranchReferenceForbidden, |
330 | - ) |
331 | from lp.testing import TestCase |
332 | |
333 | |
334 | |
335 | === modified file 'lib/lp/codehosting/puller/tests/test_worker.py' |
336 | --- lib/lp/codehosting/puller/tests/test_worker.py 2011-12-11 21:52:16 +0000 |
337 | +++ lib/lp/codehosting/puller/tests/test_worker.py 2012-09-06 09:05:37 +0000 |
338 | @@ -24,6 +24,12 @@ |
339 | TestCaseWithTransport, |
340 | ) |
341 | from bzrlib.transport import get_transport |
342 | +from bzrlib.url_policy_open import ( |
343 | + AcceptAnythingPolicy, |
344 | + BadUrl, |
345 | + BranchOpenPolicy, |
346 | + BranchOpener, |
347 | + ) |
348 | |
349 | from lp.code.enums import BranchType |
350 | from lp.codehosting.puller.tests import ( |
351 | @@ -39,14 +45,8 @@ |
352 | install_worker_ui_factory, |
353 | MirroredBranchPolicy, |
354 | PullerWorkerProtocol, |
355 | - SafeBranchOpener, |
356 | WORKER_ACTIVITY_NETWORK, |
357 | ) |
358 | -from lp.codehosting.safe_open import ( |
359 | - AcceptAnythingPolicy, |
360 | - BadUrl, |
361 | - BranchOpenPolicy, |
362 | - ) |
363 | from lp.testing import TestCase |
364 | from lp.testing.factory import ( |
365 | LaunchpadObjectFactory, |
366 | @@ -88,7 +88,7 @@ |
367 | |
368 | def setUp(self): |
369 | super(TestPullerWorker, self).setUp() |
370 | - SafeBranchOpener.install_hook() |
371 | + BranchOpener.install_hook() |
372 | |
373 | def test_mirror_opener_with_stacked_on_url(self): |
374 | # A PullerWorker for a mirrored branch gets a MirroredBranchPolicy as |
375 | @@ -279,7 +279,7 @@ |
376 | |
377 | def setUp(self): |
378 | super(TestReferenceOpener, self).setUp() |
379 | - SafeBranchOpener.install_hook() |
380 | + BranchOpener.install_hook() |
381 | |
382 | def createBranchReference(self, url): |
383 | """Create a pure branch reference that points to the specified URL. |
384 | @@ -320,20 +320,20 @@ |
385 | self.assertEqual(opened_branch.base, target_branch.base) |
386 | |
387 | def testFollowReferenceValue(self): |
388 | - # SafeBranchOpener.followReference gives the reference value for |
389 | + # BranchOpener.follow_reference gives the reference value for |
390 | # a branch reference. |
391 | - opener = SafeBranchOpener(BranchOpenPolicy()) |
392 | + opener = BranchOpener(BranchOpenPolicy()) |
393 | reference_value = 'http://example.com/branch' |
394 | reference_url = self.createBranchReference(reference_value) |
395 | self.assertEqual( |
396 | - reference_value, opener.followReference(reference_url)) |
397 | + reference_value, opener.follow_reference(reference_url)) |
398 | |
399 | def testFollowReferenceNone(self): |
400 | - # SafeBranchOpener.followReference gives None for a normal branch. |
401 | + # BranchOpener.follow_reference gives None for a normal branch. |
402 | self.make_branch('repo') |
403 | branch_url = self.get_url('repo') |
404 | - opener = SafeBranchOpener(BranchOpenPolicy()) |
405 | - self.assertIs(None, opener.followReference(branch_url)) |
406 | + opener = BranchOpener(BranchOpenPolicy()) |
407 | + self.assertIs(None, opener.follow_reference(branch_url)) |
408 | |
409 | |
410 | class TestMirroredBranchPolicy(TestCase): |
411 | @@ -346,44 +346,44 @@ |
412 | def testNoFileURL(self): |
413 | policy = MirroredBranchPolicy() |
414 | self.assertRaises( |
415 | - BadUrlScheme, policy.checkOneURL, |
416 | + BadUrlScheme, policy.check_one_url, |
417 | self.factory.getUniqueURL(scheme='file')) |
418 | |
419 | def testNoUnknownSchemeURLs(self): |
420 | policy = MirroredBranchPolicy() |
421 | self.assertRaises( |
422 | - BadUrlScheme, policy.checkOneURL, |
423 | + BadUrlScheme, policy.check_one_url, |
424 | self.factory.getUniqueURL(scheme='decorator+scheme')) |
425 | |
426 | def testNoSSHURL(self): |
427 | policy = MirroredBranchPolicy() |
428 | self.assertRaises( |
429 | - BadUrlSsh, policy.checkOneURL, |
430 | + BadUrlSsh, policy.check_one_url, |
431 | self.factory.getUniqueURL(scheme='bzr+ssh')) |
432 | |
433 | def testNoSftpURL(self): |
434 | policy = MirroredBranchPolicy() |
435 | self.assertRaises( |
436 | - BadUrlSsh, policy.checkOneURL, |
437 | + BadUrlSsh, policy.check_one_url, |
438 | self.factory.getUniqueURL(scheme='sftp')) |
439 | |
440 | def testNoLaunchpadURL(self): |
441 | policy = MirroredBranchPolicy() |
442 | self.assertRaises( |
443 | - BadUrlLaunchpad, policy.checkOneURL, |
444 | + BadUrlLaunchpad, policy.check_one_url, |
445 | self.factory.getUniqueURL(host='bazaar.launchpad.dev')) |
446 | |
447 | def testNoHTTPSLaunchpadURL(self): |
448 | policy = MirroredBranchPolicy() |
449 | self.assertRaises( |
450 | - BadUrlLaunchpad, policy.checkOneURL, |
451 | + BadUrlLaunchpad, policy.check_one_url, |
452 | self.factory.getUniqueURL( |
453 | host='bazaar.launchpad.dev', scheme='https')) |
454 | |
455 | def testNoOtherHostLaunchpadURL(self): |
456 | policy = MirroredBranchPolicy() |
457 | self.assertRaises( |
458 | - BadUrlLaunchpad, policy.checkOneURL, |
459 | + BadUrlLaunchpad, policy.check_one_url, |
460 | self.factory.getUniqueURL(host='code.launchpad.dev')) |
461 | |
462 | def testLocalhost(self): |
463 | @@ -391,9 +391,9 @@ |
464 | 'codehosting', blacklisted_hostnames='localhost,127.0.0.1') |
465 | policy = MirroredBranchPolicy() |
466 | localhost_url = self.factory.getUniqueURL(host='localhost') |
467 | - self.assertRaises(BadUrl, policy.checkOneURL, localhost_url) |
468 | + self.assertRaises(BadUrl, policy.check_one_url, localhost_url) |
469 | localhost_url = self.factory.getUniqueURL(host='127.0.0.1') |
470 | - self.assertRaises(BadUrl, policy.checkOneURL, localhost_url) |
471 | + self.assertRaises(BadUrl, policy.check_one_url, localhost_url) |
472 | |
473 | def test_no_stacked_on_url(self): |
474 | # By default, a MirroredBranchPolicy does not stack branches. |
475 | @@ -497,7 +497,7 @@ |
476 | |
477 | def setUp(self): |
478 | super(TestWorkerProgressReporting, self).setUp() |
479 | - SafeBranchOpener.install_hook() |
480 | + BranchOpener.install_hook() |
481 | self.saved_factory = bzrlib.ui.ui_factory |
482 | self.disable_directory_isolation() |
483 | self.addCleanup(setattr, bzrlib.ui, 'ui_factory', self.saved_factory) |
484 | |
485 | === modified file 'lib/lp/codehosting/puller/tests/test_worker_formats.py' |
486 | --- lib/lp/codehosting/puller/tests/test_worker_formats.py 2011-09-26 07:33:00 +0000 |
487 | +++ lib/lp/codehosting/puller/tests/test_worker_formats.py 2012-09-06 09:05:37 +0000 |
488 | @@ -15,10 +15,10 @@ |
489 | from bzrlib.repofmt.knitpack_repo import RepositoryFormatKnitPack5 |
490 | from bzrlib.repofmt.knitrepo import RepositoryFormatKnit1 |
491 | from bzrlib.tests.per_repository import TestCaseWithRepository |
492 | +from bzrlib.url_policy_open import BranchOpener |
493 | |
494 | import lp.codehosting # For bzr plugins. |
495 | from lp.codehosting.puller.tests import PullerWorkerMixin |
496 | -from lp.codehosting.safe_open import SafeBranchOpener |
497 | from lp.codehosting.tests.helpers import LoomTestMixin |
498 | |
499 | |
500 | @@ -29,7 +29,7 @@ |
501 | TestCaseWithRepository.setUp(self) |
502 | # make_bzrdir relies on this being a relative filesystem path. |
503 | self._source_branch_path = 'source-branch' |
504 | - SafeBranchOpener.install_hook() |
505 | + BranchOpener.install_hook() |
506 | self.worker = self.makePullerWorker( |
507 | self.get_url(self._source_branch_path), |
508 | self.get_url('dest-path')) |
509 | |
510 | === modified file 'lib/lp/codehosting/puller/worker.py' |
511 | --- lib/lp/codehosting/puller/worker.py 2012-06-29 08:40:05 +0000 |
512 | +++ lib/lp/codehosting/puller/worker.py 2012-09-06 09:05:37 +0000 |
513 | @@ -27,6 +27,14 @@ |
514 | from bzrlib.transport import get_transport |
515 | import bzrlib.ui |
516 | from bzrlib.ui import SilentUIFactory |
517 | +from bzrlib.url_policy_open import ( |
518 | + BadUrl, |
519 | + BranchLoopError, |
520 | + BranchOpenPolicy, |
521 | + BranchReferenceForbidden, |
522 | + BranchOpener, |
523 | + ) |
524 | + |
525 | from lazr.uri import ( |
526 | InvalidURIError, |
527 | URI, |
528 | @@ -39,13 +47,6 @@ |
529 | from lp.code.enums import BranchType |
530 | from lp.codehosting.bzrutils import identical_formats |
531 | from lp.codehosting.puller import get_lock_id_for_branch_id |
532 | -from lp.codehosting.safe_open import ( |
533 | - BadUrl, |
534 | - BranchLoopError, |
535 | - BranchOpenPolicy, |
536 | - BranchReferenceForbidden, |
537 | - SafeBranchOpener, |
538 | - ) |
539 | from lp.services.config import config |
540 | from lp.services.webapp import errorlog |
541 | |
542 | @@ -75,7 +76,7 @@ |
543 | """Found a URL with an untrusted scheme.""" |
544 | |
545 | def __init__(self, scheme, url): |
546 | - BadUrl.__init__(self, scheme, url) |
547 | + BadUrl.__init__(self, url) |
548 | self.scheme = scheme |
549 | |
550 | |
551 | @@ -195,7 +196,7 @@ |
552 | """ |
553 | self.policy = policy |
554 | self.protocol = protocol |
555 | - self.opener = SafeBranchOpener(policy) |
556 | + self.opener = BranchOpener(policy) |
557 | if log is not None: |
558 | self.log = log |
559 | else: |
560 | @@ -213,7 +214,7 @@ |
561 | URL must point to a writable location. |
562 | :return: The destination branch. |
563 | """ |
564 | - return self.opener.runWithTransformFallbackLocationHookInstalled( |
565 | + return self.opener.run_with_transform_fallback_location_hook_installed( |
566 | self.policy.createDestinationBranch, source_branch, |
567 | destination_url) |
568 | |
569 | @@ -552,8 +553,8 @@ |
570 | return None |
571 | return self.stacked_on_url |
572 | |
573 | - def shouldFollowReferences(self): |
574 | - """See `BranchOpenPolicy.shouldFollowReferences`. |
575 | + def should_follow_references(self): |
576 | + """See `BranchOpenPolicy.should_follow_references`. |
577 | |
578 | We traverse branch references for MIRRORED branches because they |
579 | provide a useful redirection mechanism and we want to be consistent |
580 | @@ -561,16 +562,16 @@ |
581 | """ |
582 | return True |
583 | |
584 | - def transformFallbackLocation(self, branch, url): |
585 | - """See `BranchOpenPolicy.transformFallbackLocation`. |
586 | + def transform_fallback_location(self, branch, url): |
587 | + """See `BranchOpenPolicy.transform_fallback_location`. |
588 | |
589 | For mirrored branches, we stack on whatever the remote branch claims |
590 | to stack on, but this URL still needs to be checked. |
591 | """ |
592 | return urlutils.join(branch.base, url), True |
593 | |
594 | - def checkOneURL(self, url): |
595 | - """See `BranchOpenPolicy.checkOneURL`. |
596 | + def check_one_url(self, url): |
597 | + """See `BranchOpenPolicy.check_one_url`. |
598 | |
599 | We refuse to mirror from Launchpad or a ssh-like or file URL. |
600 | """ |
601 | @@ -619,23 +620,23 @@ |
602 | break |
603 | return Branch.open_from_transport(dest_transport) |
604 | |
605 | - def shouldFollowReferences(self): |
606 | - """See `BranchOpenerPolicy.shouldFollowReferences`. |
607 | + def should_follow_references(self): |
608 | + """See `BranchOpenerPolicy.should_follow_references`. |
609 | |
610 | We do not traverse references for IMPORTED branches because the |
611 | code-import system should never produce branch references. |
612 | """ |
613 | return False |
614 | |
615 | - def transformFallbackLocation(self, branch, url): |
616 | - """See `BranchOpenerPolicy.transformFallbackLocation`. |
617 | + def transform_fallback_location(self, branch, url): |
618 | + """See `BranchOpenerPolicy.transform_fallback_location`. |
619 | |
620 | Import branches should not be stacked, ever. |
621 | """ |
622 | raise AssertionError("Import branch unexpectedly stacked!") |
623 | |
624 | - def checkOneURL(self, url): |
625 | - """See `BranchOpenerPolicy.checkOneURL`. |
626 | + def check_one_url(self, url): |
627 | + """See `BranchOpenerPolicy.check_one_url`. |
628 | |
629 | If the URL we are mirroring from does not start how we expect the pull |
630 | URLs of import branches to start, something has gone badly wrong, so |
631 | |
632 | === removed file 'lib/lp/codehosting/safe_open.py' |
633 | --- lib/lp/codehosting/safe_open.py 2012-08-21 17:34:42 +0000 |
634 | +++ lib/lp/codehosting/safe_open.py 1970-01-01 00:00:00 +0000 |
635 | @@ -1,354 +0,0 @@ |
636 | -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the |
637 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
638 | - |
639 | -"""Safe branch opening.""" |
640 | - |
641 | -__metaclass__ = type |
642 | - |
643 | -import threading |
644 | - |
645 | -from bzrlib import ( |
646 | - errors, |
647 | - trace, |
648 | - urlutils, |
649 | - ) |
650 | -from bzrlib.branch import Branch |
651 | -from bzrlib.bzrdir import ( |
652 | - BzrProber, |
653 | - RemoteBzrProber, |
654 | - ) |
655 | -from bzrlib.transport import ( |
656 | - do_catching_redirections, |
657 | - get_transport, |
658 | - ) |
659 | -from lazr.uri import URI |
660 | - |
661 | - |
662 | -__all__ = [ |
663 | - 'AcceptAnythingPolicy', |
664 | - 'BadUrl', |
665 | - 'BlacklistPolicy', |
666 | - 'BranchLoopError', |
667 | - 'BranchOpenPolicy', |
668 | - 'BranchReferenceForbidden', |
669 | - 'SafeBranchOpener', |
670 | - 'WhitelistPolicy', |
671 | - 'safe_open', |
672 | - ] |
673 | - |
674 | - |
675 | -# TODO JelmerVernooij 2011-08-06: This module is generic enough to be |
676 | -# in bzrlib, and may be of use to others. bug=850843 |
677 | - |
678 | -# These are the default probers that SafeBranchOpener will try to use, |
679 | -# unless a different set was specified. |
680 | - |
681 | -DEFAULT_PROBERS = [BzrProber, RemoteBzrProber] |
682 | - |
683 | - |
684 | -class BadUrl(Exception): |
685 | - """Tried to access a branch from a bad URL.""" |
686 | - |
687 | - |
688 | -class BranchReferenceForbidden(Exception): |
689 | - """Trying to mirror a branch reference and the branch type does not allow |
690 | - references. |
691 | - """ |
692 | - |
693 | - |
694 | -class BranchLoopError(Exception): |
695 | - """Encountered a branch cycle. |
696 | - |
697 | - A URL may point to a branch reference or it may point to a stacked branch. |
698 | - In either case, it's possible for there to be a cycle in these references, |
699 | - and this exception is raised when we detect such a cycle. |
700 | - """ |
701 | - |
702 | - |
703 | -class BranchOpenPolicy: |
704 | - """Policy on how to open branches. |
705 | - |
706 | - In particular, a policy determines which branches are safe to open by |
707 | - checking their URLs and deciding whether or not to follow branch |
708 | - references. |
709 | - """ |
710 | - |
711 | - def shouldFollowReferences(self): |
712 | - """Whether we traverse references when mirroring. |
713 | - |
714 | - Subclasses must override this method. |
715 | - |
716 | - If we encounter a branch reference and this returns false, an error is |
717 | - raised. |
718 | - |
719 | - :returns: A boolean to indicate whether to follow a branch reference. |
720 | - """ |
721 | - raise NotImplementedError(self.shouldFollowReferences) |
722 | - |
723 | - def transformFallbackLocation(self, branch, url): |
724 | - """Validate, maybe modify, 'url' to be used as a stacked-on location. |
725 | - |
726 | - :param branch: The branch that is being opened. |
727 | - :param url: The URL that the branch provides for its stacked-on |
728 | - location. |
729 | - :return: (new_url, check) where 'new_url' is the URL of the branch to |
730 | - actually open and 'check' is true if 'new_url' needs to be |
731 | - validated by checkAndFollowBranchReference. |
732 | - """ |
733 | - raise NotImplementedError(self.transformFallbackLocation) |
734 | - |
735 | - def checkOneURL(self, url): |
736 | - """Check the safety of the source URL. |
737 | - |
738 | - Subclasses must override this method. |
739 | - |
740 | - :param url: The source URL to check. |
741 | - :raise BadUrl: subclasses are expected to raise this or a subclass |
742 | - when it finds a URL it deems to be unsafe. |
743 | - """ |
744 | - raise NotImplementedError(self.checkOneURL) |
745 | - |
746 | - |
747 | -class BlacklistPolicy(BranchOpenPolicy): |
748 | - """Branch policy that forbids certain URLs.""" |
749 | - |
750 | - def __init__(self, should_follow_references, unsafe_urls=None): |
751 | - if unsafe_urls is None: |
752 | - unsafe_urls = set() |
753 | - self._unsafe_urls = unsafe_urls |
754 | - self._should_follow_references = should_follow_references |
755 | - |
756 | - def shouldFollowReferences(self): |
757 | - return self._should_follow_references |
758 | - |
759 | - def checkOneURL(self, url): |
760 | - if url in self._unsafe_urls: |
761 | - raise BadUrl(url) |
762 | - |
763 | - def transformFallbackLocation(self, branch, url): |
764 | - """See `BranchOpenPolicy.transformFallbackLocation`. |
765 | - |
766 | - This class is not used for testing our smarter stacking features so we |
767 | - just do the simplest thing: return the URL that would be used anyway |
768 | - and don't check it. |
769 | - """ |
770 | - return urlutils.join(branch.base, url), False |
771 | - |
772 | - |
773 | -class AcceptAnythingPolicy(BlacklistPolicy): |
774 | - """Accept anything, to make testing easier.""" |
775 | - |
776 | - def __init__(self): |
777 | - super(AcceptAnythingPolicy, self).__init__(True, set()) |
778 | - |
779 | - |
780 | -class WhitelistPolicy(BranchOpenPolicy): |
781 | - """Branch policy that only allows certain URLs.""" |
782 | - |
783 | - def __init__(self, should_follow_references, allowed_urls=None, |
784 | - check=False): |
785 | - if allowed_urls is None: |
786 | - allowed_urls = [] |
787 | - self.allowed_urls = set(url.rstrip('/') for url in allowed_urls) |
788 | - self.check = check |
789 | - |
790 | - def shouldFollowReferences(self): |
791 | - return self._should_follow_references |
792 | - |
793 | - def checkOneURL(self, url): |
794 | - if url.rstrip('/') not in self.allowed_urls: |
795 | - raise BadUrl(url) |
796 | - |
797 | - def transformFallbackLocation(self, branch, url): |
798 | - """See `BranchOpenPolicy.transformFallbackLocation`. |
799 | - |
800 | - Here we return the URL that would be used anyway and optionally check |
801 | - it. |
802 | - """ |
803 | - return urlutils.join(branch.base, url), self.check |
804 | - |
805 | - |
806 | -class SingleSchemePolicy(BranchOpenPolicy): |
807 | - """Branch open policy that rejects URLs not on the given scheme.""" |
808 | - |
809 | - def __init__(self, allowed_scheme): |
810 | - self.allowed_scheme = allowed_scheme |
811 | - |
812 | - def shouldFollowReferences(self): |
813 | - return True |
814 | - |
815 | - def transformFallbackLocation(self, branch, url): |
816 | - return urlutils.join(branch.base, url), True |
817 | - |
818 | - def checkOneURL(self, url): |
819 | - """Check that `url` is safe to open.""" |
820 | - if URI(url).scheme != self.allowed_scheme: |
821 | - raise BadUrl(url) |
822 | - |
823 | - |
824 | -class SafeBranchOpener(object): |
825 | - """Safe branch opener. |
826 | - |
827 | - All locations that are opened (stacked-on branches, references) are |
828 | - checked against a policy object. |
829 | - |
830 | - The policy object is expected to have the following methods: |
831 | - * checkOneURL |
832 | - * shouldFollowReferences |
833 | - * transformFallbackLocation |
834 | - """ |
835 | - |
836 | - _threading_data = threading.local() |
837 | - |
838 | - def __init__(self, policy, probers=None): |
839 | - """Create a new SafeBranchOpener. |
840 | - |
841 | - :param policy: The opener policy to use. |
842 | - :param probers: Optional list of probers to allow. |
843 | - Defaults to local and remote bzr probers. |
844 | - """ |
845 | - self.policy = policy |
846 | - self._seen_urls = set() |
847 | - if probers is None: |
848 | - self.probers = list(DEFAULT_PROBERS) |
849 | - else: |
850 | - self.probers = probers |
851 | - |
852 | - @classmethod |
853 | - def install_hook(cls): |
854 | - """Install the ``transformFallbackLocation`` hook. |
855 | - |
856 | - This is done at module import time, but transformFallbackLocationHook |
857 | - doesn't do anything unless the `_active_openers` threading.Local |
858 | - object has a 'opener' attribute in this thread. |
859 | - |
860 | - This is in a module-level function rather than performed at module |
861 | - level so that it can be called in setUp for testing `SafeBranchOpener` |
862 | - as bzrlib.tests.TestCase.setUp clears hooks. |
863 | - """ |
864 | - Branch.hooks.install_named_hook( |
865 | - 'transform_fallback_location', |
866 | - cls.transformFallbackLocationHook, |
867 | - 'SafeBranchOpener.transformFallbackLocationHook') |
868 | - |
869 | - def checkAndFollowBranchReference(self, url): |
870 | - """Check URL (and possibly the referenced URL) for safety. |
871 | - |
872 | - This method checks that `url` passes the policy's `checkOneURL` |
873 | - method, and if `url` refers to a branch reference, it checks whether |
874 | - references are allowed and whether the reference's URL passes muster |
875 | - also -- recursively, until a real branch is found. |
876 | - |
877 | - :param url: URL to check |
878 | - :raise BranchLoopError: If the branch references form a loop. |
879 | - :raise BranchReferenceForbidden: If this opener forbids branch |
880 | - references. |
881 | - """ |
882 | - while True: |
883 | - if url in self._seen_urls: |
884 | - raise BranchLoopError() |
885 | - self._seen_urls.add(url) |
886 | - self.policy.checkOneURL(url) |
887 | - next_url = self.followReference(url) |
888 | - if next_url is None: |
889 | - return url |
890 | - url = next_url |
891 | - if not self.policy.shouldFollowReferences(): |
892 | - raise BranchReferenceForbidden(url) |
893 | - |
894 | - @classmethod |
895 | - def transformFallbackLocationHook(cls, branch, url): |
896 | - """Installed as the 'transform_fallback_location' Branch hook. |
897 | - |
898 | - This method calls `transformFallbackLocation` on the policy object and |
899 | - either returns the url it provides or passes it back to |
900 | - checkAndFollowBranchReference. |
901 | - """ |
902 | - try: |
903 | - opener = getattr(cls._threading_data, "opener") |
904 | - except AttributeError: |
905 | - return url |
906 | - new_url, check = opener.policy.transformFallbackLocation(branch, url) |
907 | - if check: |
908 | - return opener.checkAndFollowBranchReference(new_url) |
909 | - else: |
910 | - return new_url |
911 | - |
912 | - def runWithTransformFallbackLocationHookInstalled( |
913 | - self, callable, *args, **kw): |
914 | - assert (self.transformFallbackLocationHook in |
915 | - Branch.hooks['transform_fallback_location']) |
916 | - self._threading_data.opener = self |
917 | - try: |
918 | - return callable(*args, **kw) |
919 | - finally: |
920 | - del self._threading_data.opener |
921 | - # We reset _seen_urls here to avoid multiple calls to open giving |
922 | - # spurious loop exceptions. |
923 | - self._seen_urls = set() |
924 | - |
925 | - def followReference(self, url): |
926 | - """Get the branch-reference value at the specified url. |
927 | - |
928 | - This exists as a separate method only to be overridden in unit tests. |
929 | - """ |
930 | - bzrdir = self._open_dir(url) |
931 | - return bzrdir.get_branch_reference() |
932 | - |
933 | - def _open_dir(self, url): |
934 | - """Simple BzrDir.open clone that only uses specific probers. |
935 | - |
936 | - :param url: URL to open |
937 | - :return: ControlDir instance |
938 | - """ |
939 | - def redirected(transport, e, redirection_notice): |
940 | - self.policy.checkOneURL(e.target) |
941 | - redirected_transport = transport._redirected_to( |
942 | - e.source, e.target) |
943 | - if redirected_transport is None: |
944 | - raise errors.NotBranchError(e.source) |
945 | - trace.note('%s is%s redirected to %s', |
946 | - transport.base, e.permanently, redirected_transport.base) |
947 | - return redirected_transport |
948 | - |
949 | - def find_format(transport): |
950 | - last_error = errors.NotBranchError(transport.base) |
951 | - for prober_kls in self.probers: |
952 | - prober = prober_kls() |
953 | - try: |
954 | - return transport, prober.probe_transport(transport) |
955 | - except errors.NotBranchError as e: |
956 | - last_error = e |
957 | - else: |
958 | - raise last_error |
959 | - transport = get_transport(url) |
960 | - transport, format = do_catching_redirections(find_format, transport, |
961 | - redirected) |
962 | - return format.open(transport) |
963 | - |
964 | - def open(self, url, ignore_fallbacks=False): |
965 | - """Open the Bazaar branch at url, first checking for safety. |
966 | - |
967 | - What safety means is defined by a subclasses `followReference` and |
968 | - `checkOneURL` methods. |
969 | - """ |
970 | - url = self.checkAndFollowBranchReference(url) |
971 | - |
972 | - def open_branch(url, ignore_fallbacks): |
973 | - dir = self._open_dir(url) |
974 | - return dir.open_branch(ignore_fallbacks=ignore_fallbacks) |
975 | - return self.runWithTransformFallbackLocationHookInstalled( |
976 | - open_branch, url, ignore_fallbacks) |
977 | - |
978 | - |
979 | -def safe_open(allowed_scheme, url, ignore_fallbacks=False): |
980 | - """Open the branch at `url`, only accessing URLs on `allowed_scheme`. |
981 | - |
982 | - :raises BadUrl: An attempt was made to open a URL that was not on |
983 | - `allowed_scheme`. |
984 | - """ |
985 | - return SafeBranchOpener(SingleSchemePolicy(allowed_scheme)).open(url, |
986 | - ignore_fallbacks=ignore_fallbacks) |
987 | - |
988 | - |
989 | -SafeBranchOpener.install_hook() |
990 | |
991 | === modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py' |
992 | --- lib/lp/codehosting/scanner/tests/test_bzrsync.py 2012-06-26 09:40:51 +0000 |
993 | +++ lib/lp/codehosting/scanner/tests/test_bzrsync.py 2012-09-06 09:05:37 +0000 |
994 | @@ -16,6 +16,7 @@ |
995 | ) |
996 | from bzrlib.tests import TestCaseWithTransport |
997 | from bzrlib.uncommit import uncommit |
998 | +from bzrlib.url_policy_open import BranchOpener |
999 | from fixtures import TempDir |
1000 | import pytz |
1001 | from storm.locals import Store |
1002 | @@ -42,7 +43,6 @@ |
1003 | read_locked, |
1004 | write_locked, |
1005 | ) |
1006 | -from lp.codehosting.safe_open import SafeBranchOpener |
1007 | from lp.codehosting.scanner.bzrsync import BzrSync |
1008 | from lp.services.config import config |
1009 | from lp.services.database.lpstorm import IStore |
1010 | @@ -82,7 +82,7 @@ |
1011 | |
1012 | def setUp(self): |
1013 | super(BzrSyncTestCase, self).setUp() |
1014 | - SafeBranchOpener.install_hook() |
1015 | + BranchOpener.install_hook() |
1016 | self.disable_directory_isolation() |
1017 | self.useBzrBranches(direct_database=True) |
1018 | self.makeFixtures() |
1019 | |
1020 | === removed file 'lib/lp/codehosting/tests/test_safe_open.py' |
1021 | --- lib/lp/codehosting/tests/test_safe_open.py 2012-09-06 00:01:38 +0000 |
1022 | +++ lib/lp/codehosting/tests/test_safe_open.py 1970-01-01 00:00:00 +0000 |
1023 | @@ -1,382 +0,0 @@ |
1024 | -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the |
1025 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
1026 | - |
1027 | -"""Tests for the safe branch open code.""" |
1028 | - |
1029 | - |
1030 | -__metaclass__ = type |
1031 | - |
1032 | -from bzrlib.branch import ( |
1033 | - Branch, |
1034 | - BranchReferenceFormat, |
1035 | - BzrBranchFormat7, |
1036 | - ) |
1037 | -from bzrlib.bzrdir import ( |
1038 | - BzrDir, |
1039 | - BzrDirMetaFormat1, |
1040 | - BzrProber, |
1041 | - ) |
1042 | -from bzrlib.controldir import ControlDirFormat |
1043 | -from bzrlib.errors import NotBranchError |
1044 | -from bzrlib.repofmt.knitpack_repo import RepositoryFormatKnitPack1 |
1045 | -from bzrlib.tests import TestCaseWithTransport |
1046 | -from bzrlib.transport import chroot |
1047 | -from lazr.uri import URI |
1048 | - |
1049 | -from lp.codehosting.safe_open import ( |
1050 | - BadUrl, |
1051 | - BlacklistPolicy, |
1052 | - BranchLoopError, |
1053 | - BranchReferenceForbidden, |
1054 | - safe_open, |
1055 | - SafeBranchOpener, |
1056 | - WhitelistPolicy, |
1057 | - ) |
1058 | -from lp.codehosting.tests.helpers import force_stacked_on_url |
1059 | -from lp.testing import TestCase |
1060 | - |
1061 | - |
1062 | -class TestSafeBranchOpenerCheckAndFollowBranchReference(TestCase): |
1063 | - """Unit tests for `SafeBranchOpener.checkAndFollowBranchReference`.""" |
1064 | - |
1065 | - def setUp(self): |
1066 | - super(TestSafeBranchOpenerCheckAndFollowBranchReference, self).setUp() |
1067 | - SafeBranchOpener.install_hook() |
1068 | - |
1069 | - class StubbedSafeBranchOpener(SafeBranchOpener): |
1070 | - """SafeBranchOpener that provides canned answers. |
1071 | - |
1072 | - We implement the methods we need to to be able to control all the |
1073 | - inputs to the `BranchMirrorer.checkSource` method, which is what is |
1074 | - being tested in this class. |
1075 | - """ |
1076 | - |
1077 | - def __init__(self, references, policy): |
1078 | - parent_cls = TestSafeBranchOpenerCheckAndFollowBranchReference |
1079 | - super(parent_cls.StubbedSafeBranchOpener, self).__init__(policy) |
1080 | - self._reference_values = {} |
1081 | - for i in range(len(references) - 1): |
1082 | - self._reference_values[references[i]] = references[i + 1] |
1083 | - self.follow_reference_calls = [] |
1084 | - |
1085 | - def followReference(self, url): |
1086 | - self.follow_reference_calls.append(url) |
1087 | - return self._reference_values[url] |
1088 | - |
1089 | - def makeBranchOpener(self, should_follow_references, references, |
1090 | - unsafe_urls=None): |
1091 | - policy = BlacklistPolicy(should_follow_references, unsafe_urls) |
1092 | - opener = self.StubbedSafeBranchOpener(references, policy) |
1093 | - return opener |
1094 | - |
1095 | - def testCheckInitialURL(self): |
1096 | - # checkSource rejects all URLs that are not allowed. |
1097 | - opener = self.makeBranchOpener(None, [], set(['a'])) |
1098 | - self.assertRaises( |
1099 | - BadUrl, opener.checkAndFollowBranchReference, 'a') |
1100 | - |
1101 | - def testNotReference(self): |
1102 | - # When branch references are forbidden, checkAndFollowBranchReference |
1103 | - # does not raise on non-references. |
1104 | - opener = self.makeBranchOpener(False, ['a', None]) |
1105 | - self.assertEquals( |
1106 | - 'a', opener.checkAndFollowBranchReference('a')) |
1107 | - self.assertEquals(['a'], opener.follow_reference_calls) |
1108 | - |
1109 | - def testBranchReferenceForbidden(self): |
1110 | - # checkAndFollowBranchReference raises BranchReferenceForbidden if |
1111 | - # branch references are forbidden and the source URL points to a |
1112 | - # branch reference. |
1113 | - opener = self.makeBranchOpener(False, ['a', 'b']) |
1114 | - self.assertRaises( |
1115 | - BranchReferenceForbidden, |
1116 | - opener.checkAndFollowBranchReference, 'a') |
1117 | - self.assertEquals(['a'], opener.follow_reference_calls) |
1118 | - |
1119 | - def testAllowedReference(self): |
1120 | - # checkAndFollowBranchReference does not raise if following references |
1121 | - # is allowed and the source URL points to a branch reference to a |
1122 | - # permitted location. |
1123 | - opener = self.makeBranchOpener(True, ['a', 'b', None]) |
1124 | - self.assertEquals( |
1125 | - 'b', opener.checkAndFollowBranchReference('a')) |
1126 | - self.assertEquals(['a', 'b'], opener.follow_reference_calls) |
1127 | - |
1128 | - def testCheckReferencedURLs(self): |
1129 | - # checkAndFollowBranchReference checks if the URL a reference points |
1130 | - # to is safe. |
1131 | - opener = self.makeBranchOpener( |
1132 | - True, ['a', 'b', None], unsafe_urls=set('b')) |
1133 | - self.assertRaises( |
1134 | - BadUrl, opener.checkAndFollowBranchReference, 'a') |
1135 | - self.assertEquals(['a'], opener.follow_reference_calls) |
1136 | - |
1137 | - def testSelfReferencingBranch(self): |
1138 | - # checkAndFollowBranchReference raises BranchReferenceLoopError if |
1139 | - # following references is allowed and the source url points to a |
1140 | - # self-referencing branch reference. |
1141 | - opener = self.makeBranchOpener(True, ['a', 'a']) |
1142 | - self.assertRaises( |
1143 | - BranchLoopError, opener.checkAndFollowBranchReference, 'a') |
1144 | - self.assertEquals(['a'], opener.follow_reference_calls) |
1145 | - |
1146 | - def testBranchReferenceLoop(self): |
1147 | - # checkAndFollowBranchReference raises BranchReferenceLoopError if |
1148 | - # following references is allowed and the source url points to a loop |
1149 | - # of branch references. |
1150 | - references = ['a', 'b', 'a'] |
1151 | - opener = self.makeBranchOpener(True, references) |
1152 | - self.assertRaises( |
1153 | - BranchLoopError, opener.checkAndFollowBranchReference, 'a') |
1154 | - self.assertEquals(['a', 'b'], opener.follow_reference_calls) |
1155 | - |
1156 | - |
1157 | -class TrackingProber(BzrProber): |
1158 | - """Subclass of BzrProber which tracks URLs it has been asked to open.""" |
1159 | - |
1160 | - seen_urls = [] |
1161 | - |
1162 | - @classmethod |
1163 | - def probe_transport(klass, transport): |
1164 | - klass.seen_urls.append(transport.base) |
1165 | - return BzrProber.probe_transport(transport) |
1166 | - |
1167 | - |
1168 | -class TestSafeBranchOpenerStacking(TestCaseWithTransport): |
1169 | - |
1170 | - def setUp(self): |
1171 | - super(TestSafeBranchOpenerStacking, self).setUp() |
1172 | - SafeBranchOpener.install_hook() |
1173 | - |
1174 | - def makeBranchOpener(self, allowed_urls, probers=None): |
1175 | - policy = WhitelistPolicy(True, allowed_urls, True) |
1176 | - return SafeBranchOpener(policy, probers) |
1177 | - |
1178 | - def makeBranch(self, path, branch_format, repository_format): |
1179 | - """Make a Bazaar branch at 'path' with the given formats.""" |
1180 | - bzrdir_format = BzrDirMetaFormat1() |
1181 | - bzrdir_format.set_branch_format(branch_format) |
1182 | - bzrdir = self.make_bzrdir(path, format=bzrdir_format) |
1183 | - repository_format.initialize(bzrdir) |
1184 | - return bzrdir.create_branch() |
1185 | - |
1186 | - def testProbers(self): |
1187 | - # Only the specified probers should be used |
1188 | - b = self.make_branch('branch') |
1189 | - opener = self.makeBranchOpener([b.base], probers=[]) |
1190 | - self.assertRaises(NotBranchError, opener.open, b.base) |
1191 | - opener = self.makeBranchOpener([b.base], probers=[BzrProber]) |
1192 | - self.assertEquals(b.base, opener.open(b.base).base) |
1193 | - |
1194 | - def testDefaultProbers(self): |
1195 | - # If no probers are specified to the constructor |
1196 | - # of SafeBranchOpener, then a safe set will be used, |
1197 | - # rather than all probers registered in bzr. |
1198 | - self.addCleanup(ControlDirFormat.unregister_prober, TrackingProber) |
1199 | - ControlDirFormat.register_prober(TrackingProber) |
1200 | - # Open a location without any branches, so that all probers are |
1201 | - # tried. |
1202 | - # First, check that the TrackingProber tracks correctly. |
1203 | - TrackingProber.seen_urls = [] |
1204 | - opener = self.makeBranchOpener(["."], probers=[TrackingProber]) |
1205 | - self.assertRaises(NotBranchError, opener.open, ".") |
1206 | - self.assertEquals(1, len(TrackingProber.seen_urls)) |
1207 | - TrackingProber.seen_urls = [] |
1208 | - # And make sure it's registered in such a way that BzrDir.open would |
1209 | - # use it. |
1210 | - self.assertRaises(NotBranchError, BzrDir.open, ".") |
1211 | - self.assertEquals(1, len(TrackingProber.seen_urls)) |
1212 | - TrackingProber.seen_urls = [] |
1213 | - # Make sure that SafeBranchOpener doesn't use it if no |
1214 | - # probers were specified |
1215 | - opener = self.makeBranchOpener(["."]) |
1216 | - self.assertRaises(NotBranchError, opener.open, ".") |
1217 | - self.assertEquals(0, len(TrackingProber.seen_urls)) |
1218 | - |
1219 | - def testAllowedURL(self): |
1220 | - # checkSource does not raise an exception for branches stacked on |
1221 | - # branches with allowed URLs. |
1222 | - stacked_on_branch = self.make_branch('base-branch') |
1223 | - stacked_branch = self.make_branch('stacked-branch') |
1224 | - stacked_branch.set_stacked_on_url(stacked_on_branch.base) |
1225 | - opener = self.makeBranchOpener( |
1226 | - [stacked_branch.base, stacked_on_branch.base]) |
1227 | - # This doesn't raise an exception. |
1228 | - opener.open(stacked_branch.base) |
1229 | - |
1230 | - def testUnstackableRepository(self): |
1231 | - # checkSource treats branches with UnstackableRepositoryFormats as |
1232 | - # being not stacked. |
1233 | - branch = self.makeBranch( |
1234 | - 'unstacked', BzrBranchFormat7(), RepositoryFormatKnitPack1()) |
1235 | - opener = self.makeBranchOpener([branch.base]) |
1236 | - # This doesn't raise an exception. |
1237 | - opener.open(branch.base) |
1238 | - |
1239 | - def testAllowedRelativeURL(self): |
1240 | - # checkSource passes on absolute urls to checkOneURL, even if the |
1241 | - # value of stacked_on_location in the config is set to a relative URL. |
1242 | - stacked_on_branch = self.make_branch('base-branch') |
1243 | - stacked_branch = self.make_branch('stacked-branch') |
1244 | - stacked_branch.set_stacked_on_url('../base-branch') |
1245 | - opener = self.makeBranchOpener( |
1246 | - [stacked_branch.base, stacked_on_branch.base]) |
1247 | - # Note that stacked_on_branch.base is not '../base-branch', it's an |
1248 | - # absolute URL. |
1249 | - self.assertNotEqual('../base-branch', stacked_on_branch.base) |
1250 | - # This doesn't raise an exception. |
1251 | - opener.open(stacked_branch.base) |
1252 | - |
1253 | - def testAllowedRelativeNested(self): |
1254 | - # Relative URLs are resolved relative to the stacked branch. |
1255 | - self.get_transport().mkdir('subdir') |
1256 | - a = self.make_branch('subdir/a') |
1257 | - b = self.make_branch('b') |
1258 | - b.set_stacked_on_url('../subdir/a') |
1259 | - c = self.make_branch('subdir/c') |
1260 | - c.set_stacked_on_url('../../b') |
1261 | - opener = self.makeBranchOpener([c.base, b.base, a.base]) |
1262 | - # This doesn't raise an exception. |
1263 | - opener.open(c.base) |
1264 | - |
1265 | - def testForbiddenURL(self): |
1266 | - # checkSource raises a BadUrl exception if a branch is stacked on a |
1267 | - # branch with a forbidden URL. |
1268 | - stacked_on_branch = self.make_branch('base-branch') |
1269 | - stacked_branch = self.make_branch('stacked-branch') |
1270 | - stacked_branch.set_stacked_on_url(stacked_on_branch.base) |
1271 | - opener = self.makeBranchOpener([stacked_branch.base]) |
1272 | - self.assertRaises(BadUrl, opener.open, stacked_branch.base) |
1273 | - |
1274 | - def testForbiddenURLNested(self): |
1275 | - # checkSource raises a BadUrl exception if a branch is stacked on a |
1276 | - # branch that is in turn stacked on a branch with a forbidden URL. |
1277 | - a = self.make_branch('a') |
1278 | - b = self.make_branch('b') |
1279 | - b.set_stacked_on_url(a.base) |
1280 | - c = self.make_branch('c') |
1281 | - c.set_stacked_on_url(b.base) |
1282 | - opener = self.makeBranchOpener([c.base, b.base]) |
1283 | - self.assertRaises(BadUrl, opener.open, c.base) |
1284 | - |
1285 | - def testSelfStackedBranch(self): |
1286 | - # checkSource raises StackingLoopError if a branch is stacked on |
1287 | - # itself. This avoids infinite recursion errors. |
1288 | - a = self.make_branch('a') |
1289 | - force_stacked_on_url(a, a.base) |
1290 | - opener = self.makeBranchOpener([a.base]) |
1291 | - self.assertRaises(BranchLoopError, opener.open, a.base) |
1292 | - |
1293 | - def testLoopStackedBranch(self): |
1294 | - # checkSource raises StackingLoopError if a branch is stacked in such |
1295 | - # a way so that it is ultimately stacked on itself. e.g. a stacked on |
1296 | - # b stacked on a. |
1297 | - a = self.make_branch('a') |
1298 | - b = self.make_branch('b') |
1299 | - a.set_stacked_on_url(b.base) |
1300 | - b.set_stacked_on_url(a.base) |
1301 | - opener = self.makeBranchOpener([a.base, b.base]) |
1302 | - self.assertRaises(BranchLoopError, opener.open, a.base) |
1303 | - self.assertRaises(BranchLoopError, opener.open, b.base) |
1304 | - |
1305 | - def testCustomOpener(self): |
1306 | - # A custom function for opening a control dir can be specified. |
1307 | - a = self.make_branch('a') |
1308 | - b = self.make_branch('b') |
1309 | - b.set_stacked_on_url(a.base) |
1310 | - |
1311 | - TrackingProber.seen_urls = [] |
1312 | - opener = self.makeBranchOpener( |
1313 | - [a.base, b.base], probers=[TrackingProber]) |
1314 | - opener.open(b.base) |
1315 | - self.assertEquals( |
1316 | - set(TrackingProber.seen_urls), set([b.base, a.base])) |
1317 | - |
1318 | - def testCustomOpenerWithBranchReference(self): |
1319 | - # A custom function for opening a control dir can be specified. |
1320 | - a = self.make_branch('a') |
1321 | - b_dir = self.make_bzrdir('b') |
1322 | - b = BranchReferenceFormat().initialize(b_dir, target_branch=a) |
1323 | - TrackingProber.seen_urls = [] |
1324 | - opener = self.makeBranchOpener( |
1325 | - [a.base, b.base], probers=[TrackingProber]) |
1326 | - opener.open(b.base) |
1327 | - self.assertEquals( |
1328 | - set(TrackingProber.seen_urls), set([b.base, a.base])) |
1329 | - |
1330 | - def test_ignore_fallbacks(self): |
1331 | - """"Cross-format stacking doesn't error with ignore_fallbacks.""" |
1332 | - stacked, stacked_on = make_cross_format_stacked(self) |
1333 | - opener = self.makeBranchOpener([stacked.base, stacked_on.base]) |
1334 | - opener.open(stacked.base, ignore_fallbacks=True) |
1335 | - |
1336 | - |
1337 | -def make_cross_format_stacked(test_case): |
1338 | - test_case.get_transport().mkdir('inside') |
1339 | - stacked = test_case.make_branch('inside/stacked', format='1.6') |
1340 | - stacked_on = test_case.make_branch('inside/stacked-on', format='2a') |
1341 | - force_stacked_on_url(stacked, stacked_on.base) |
1342 | - return stacked, stacked_on |
1343 | - |
1344 | - |
1345 | -class TestSafeOpen(TestCaseWithTransport): |
1346 | - """Tests for `safe_open`.""" |
1347 | - |
1348 | - def setUp(self): |
1349 | - super(TestSafeOpen, self).setUp() |
1350 | - SafeBranchOpener.install_hook() |
1351 | - |
1352 | - def test_hook_does_not_interfere(self): |
1353 | - # The transform_fallback_location hook does not interfere with regular |
1354 | - # stacked branch access outside of safe_open. |
1355 | - self.make_branch('stacked') |
1356 | - self.make_branch('stacked-on') |
1357 | - Branch.open('stacked').set_stacked_on_url('../stacked-on') |
1358 | - Branch.open('stacked') |
1359 | - |
1360 | - def get_chrooted_scheme(self, relpath): |
1361 | - """Create a server that is chrooted to `relpath`. |
1362 | - |
1363 | - :return: ``(scheme, get_url)`` where ``scheme`` is the scheme of the |
1364 | - chroot server and ``get_url`` returns URLs on said server. |
1365 | - """ |
1366 | - transport = self.get_transport(relpath) |
1367 | - chroot_server = chroot.ChrootServer(transport) |
1368 | - chroot_server.start_server() |
1369 | - self.addCleanup(chroot_server.stop_server) |
1370 | - |
1371 | - def get_url(relpath): |
1372 | - return chroot_server.get_url() + relpath |
1373 | - |
1374 | - return URI(chroot_server.get_url()).scheme, get_url |
1375 | - |
1376 | - def test_stacked_within_scheme(self): |
1377 | - # A branch that is stacked on a URL of the same scheme is safe to |
1378 | - # open. |
1379 | - self.get_transport().mkdir('inside') |
1380 | - self.make_branch('inside/stacked') |
1381 | - self.make_branch('inside/stacked-on') |
1382 | - scheme, get_chrooted_url = self.get_chrooted_scheme('inside') |
1383 | - Branch.open(get_chrooted_url('stacked')).set_stacked_on_url( |
1384 | - get_chrooted_url('stacked-on')) |
1385 | - safe_open(scheme, get_chrooted_url('stacked')) |
1386 | - |
1387 | - def test_stacked_outside_scheme(self): |
1388 | - # A branch that is stacked on a URL that is not of the same scheme is |
1389 | - # not safe to open. |
1390 | - self.get_transport().mkdir('inside') |
1391 | - self.get_transport().mkdir('outside') |
1392 | - self.make_branch('inside/stacked') |
1393 | - self.make_branch('outside/stacked-on') |
1394 | - scheme, get_chrooted_url = self.get_chrooted_scheme('inside') |
1395 | - Branch.open(get_chrooted_url('stacked')).set_stacked_on_url( |
1396 | - self.get_url('outside/stacked-on')) |
1397 | - self.assertRaises( |
1398 | - BadUrl, safe_open, scheme, get_chrooted_url('stacked')) |
1399 | - |
1400 | - def test_ignore_fallbacks(self): |
1401 | - """"Cross-format stacking doesn't error with ignore_fallbacks.""" |
1402 | - scheme, get_chrooted_url = self.get_chrooted_scheme('inside') |
1403 | - stacked, stacked_on = make_cross_format_stacked(self) |
1404 | - force_stacked_on_url(stacked, get_chrooted_url('stacked-on')) |
1405 | - safe_open(scheme, get_chrooted_url('stacked'), ignore_fallbacks=True) |
1406 | |
1407 | === modified file 'scripts/code-import-worker.py' |
1408 | --- scripts/code-import-worker.py 2012-01-01 03:13:08 +0000 |
1409 | +++ scripts/code-import-worker.py 2012-09-06 09:05:37 +0000 |
1410 | @@ -22,6 +22,7 @@ |
1411 | import sys |
1412 | |
1413 | from bzrlib.transport import get_transport |
1414 | +from bzrlib.url_policy_open import AcceptAnythingPolicy |
1415 | |
1416 | from lp.codehosting.codeimport.worker import ( |
1417 | BzrImportWorker, |
1418 | @@ -33,7 +34,6 @@ |
1419 | GitImportWorker, |
1420 | HgImportWorker, |
1421 | ) |
1422 | -from lp.codehosting.safe_open import AcceptAnythingPolicy |
1423 | from lp.services import scripts |
1424 | from lp.services.config import config |
1425 |
Jelmer--
This looks good, thanks.
One thing: On line 240 of your diff, you have 'svn' listed twice in the list of
allowed_schemes. I assume that's just a typo/editing artifact.