Launchpad itself

Merge lp:~mwhudson/launchpad/unicod-branch-names-bug-449528 into lp:launchpad

  1. unicod-branch-names-bug-449528
  2. Merge into devel
Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~mwhudson/launchpad/unicod-branch-names-bug-449528
Merge into: lp:launchpad
Diff against target: 396 lines (+121/-29)
8 files modified
lib/lp/code/doc/branch-xmlrpc.txt (+2/-2)
lib/lp/code/model/branch.py (+1/-1)
lib/lp/code/xmlrpc/branch.py (+11/-8)
lib/lp/code/xmlrpc/codehosting.py (+11/-6)
lib/lp/code/xmlrpc/tests/test_branch.py (+63/-6)
lib/lp/code/xmlrpc/tests/test_codehosting.py (+22/-1)
lib/lp/codehosting/inmemory.py (+7/-4)
lib/lp/codehosting/vfs/branchfs.py (+4/-1)
To merge this branch: bzr merge lp:~mwhudson/launchpad/unicod-branch-names-bug-449528
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+19572@code.launchpad.net

Commit message

Fix some OOPSes that can result from attempts to push branches containing non-ASCII names.

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Hi there,

This branch fixes various explosions you can get when trying to use non-ascii characters in branch names. We don't allow this, but we should fail gracefully and this branch tries to get us there.

There are various things that conspire to making this annoying:

 * xmlrpclib papers over the difference between str and unicode to the extent that if you send a utf-8 encoded str containing non-ascii characters, the other side will get a unicode string.
   * in particular this applied to the 'faultString' attribute of Faults.
 * Python exceptions containing non-ascii unicode strings are a bit like dodgy hand grenades, at least in Python 2.5: they tend to blow up if you touch them.
 * There turned out to be bugs in bzr in this area, now fixed in the version we are using.

Anyway, this branch fixes most problems I found. I recommend running make run_all and trying to push branches named things like "bzr+ssh://<email address hidden>/~m%c2%a9rk/gnome-trminal/dd".

This branch has been sitting around in my repo for ages. I'll be happy to get it done!

Cheers,
mwh

Revision history for this message
Tim Penhey (thumper) wrote :

Looks good to me. I trust you've tried pushing locally - I'm not going to.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/doc/branch-xmlrpc.txt'
--- lib/lp/code/doc/branch-xmlrpc.txt 2009-08-13 15:12:16 +0000
+++ lib/lp/code/doc/branch-xmlrpc.txt 2010-02-22 01:48:21 +0000
@@ -417,8 +417,8 @@
417 >>> results = branchset_api.resolve_lp_path('~mark/+junk/doesntexist')417 >>> results = branchset_api.resolve_lp_path('~mark/+junk/doesntexist')
418 >>> for url in results['urls']:418 >>> for url in results['urls']:
419 ... print url419 ... print url
420 bzr+ssh://bazaar.launchpad.dev/~mark/+junk/doesntexist420 bzr+ssh://bazaar.launchpad.dev/~mark/%2Bjunk/doesntexist
421 http://bazaar.launchpad.dev/~mark/+junk/doesntexist421 http://bazaar.launchpad.dev/~mark/%2Bjunk/doesntexist
422422
423423
424But if the project or the user doesn't exist, Launchpad will return a fault:424But if the project or the user doesn't exist, Launchpad will return a fault:
425425
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2010-02-16 20:36:58 +0000
+++ lib/lp/code/model/branch.py 2010-02-22 01:48:21 +0000
@@ -1309,7 +1309,7 @@
1309 accepted_schemes.add('sftp')1309 accepted_schemes.add('sftp')
1310 assert scheme in accepted_schemes, "Unknown scheme: %s" % scheme1310 assert scheme in accepted_schemes, "Unknown scheme: %s" % scheme
1311 host = URI(config.codehosting.supermirror_root).host1311 host = URI(config.codehosting.supermirror_root).host
1312 path = '/' + unique_name1312 path = '/' + urlutils.escape(unique_name)
1313 if suffix is not None:1313 if suffix is not None:
1314 path = os.path.join(path, suffix)1314 path = os.path.join(path, suffix)
1315 return str(URI(scheme=scheme, host=host, path=path))1315 return str(URI(scheme=scheme, host=host, path=path))
13161316
=== modified file 'lib/lp/code/xmlrpc/branch.py'
--- lib/lp/code/xmlrpc/branch.py 2010-02-16 20:36:58 +0000
+++ lib/lp/code/xmlrpc/branch.py 2010-02-22 01:48:21 +0000
@@ -12,6 +12,8 @@
12 'PublicCodehostingAPI']12 'PublicCodehostingAPI']
1313
1414
15from bzrlib import urlutils
16
15from zope.component import getUtility17from zope.component import getUtility
16from zope.interface import Interface, implements18from zope.interface import Interface, implements
1719
@@ -100,7 +102,7 @@
100102
101 try:103 try:
102 unicode_branch_url = branch_url.decode('utf-8')104 unicode_branch_url = branch_url.decode('utf-8')
103 url = IBranch['url'].validate(unicode_branch_url)105 IBranch['url'].validate(unicode_branch_url)
104 except LaunchpadValidationError, exc:106 except LaunchpadValidationError, exc:
105 return faults.InvalidBranchUrl(branch_url, exc)107 return faults.InvalidBranchUrl(branch_url, exc)
106108
@@ -238,23 +240,24 @@
238 # the model code(blech) or some automated way of reraising as faults240 # the model code(blech) or some automated way of reraising as faults
239 # or using a narrower range of faults (e.g. only one "NoSuch" fault).241 # or using a narrower range of faults (e.g. only one "NoSuch" fault).
240 except InvalidProductName, e:242 except InvalidProductName, e:
241 raise faults.InvalidProductIdentifier(e.name)243 raise faults.InvalidProductIdentifier(urlutils.escape(e.name))
242 except NoSuchProductSeries, e:244 except NoSuchProductSeries, e:
243 raise faults.NoSuchProductSeries(e.name, e.product)245 raise faults.NoSuchProductSeries(
246 urlutils.escape(e.name), e.product)
244 except NoSuchPerson, e:247 except NoSuchPerson, e:
245 raise faults.NoSuchPersonWithName(e.name)248 raise faults.NoSuchPersonWithName(urlutils.escape(e.name))
246 except NoSuchProduct, e:249 except NoSuchProduct, e:
247 raise faults.NoSuchProduct(e.name)250 raise faults.NoSuchProduct(urlutils.escape(e.name))
248 except NoSuchDistroSeries, e:251 except NoSuchDistroSeries, e:
249 raise faults.NoSuchDistroSeries(e.name)252 raise faults.NoSuchDistroSeries(urlutils.escape(e.name))
250 except NoSuchSourcePackageName, e:253 except NoSuchSourcePackageName, e:
251 raise faults.NoSuchSourcePackageName(e.name)254 raise faults.NoSuchSourcePackageName(urlutils.escape(e.name))
252 except NoLinkedBranch, e:255 except NoLinkedBranch, e:
253 raise faults.NoLinkedBranch(e.component)256 raise faults.NoLinkedBranch(e.component)
254 except CannotHaveLinkedBranch, e:257 except CannotHaveLinkedBranch, e:
255 raise faults.CannotHaveLinkedBranch(e.component)258 raise faults.CannotHaveLinkedBranch(e.component)
256 except InvalidNamespace, e:259 except InvalidNamespace, e:
257 raise faults.InvalidBranchUniqueName(e.name)260 raise faults.InvalidBranchUniqueName(urlutils.escape(e.name))
258 return self._getResultDict(branch, suffix, supported_schemes)261 return self._getResultDict(branch, suffix, supported_schemes)
259262
260 def resolve_lp_path(self, path):263 def resolve_lp_path(self, path):
261264
=== modified file 'lib/lp/code/xmlrpc/codehosting.py'
--- lib/lp/code/xmlrpc/codehosting.py 2009-10-19 05:40:52 +0000
+++ lib/lp/code/xmlrpc/codehosting.py 2010-02-22 01:48:21 +0000
@@ -12,6 +12,7 @@
1212
1313
14import datetime14import datetime
15import urllib
1516
16import pytz17import pytz
1718
@@ -232,7 +233,7 @@
232 def create_branch(requester):233 def create_branch(requester):
233 if not branch_path.startswith('/'):234 if not branch_path.startswith('/'):
234 return faults.InvalidPath(branch_path)235 return faults.InvalidPath(branch_path)
235 escaped_path = unescape(branch_path.strip('/')).encode('utf-8')236 escaped_path = unescape(branch_path.strip('/'))
236 try:237 try:
237 namespace_name, branch_name = split_unique_name(escaped_path)238 namespace_name, branch_name = split_unique_name(escaped_path)
238 except ValueError:239 except ValueError:
@@ -254,7 +255,12 @@
254 try:255 try:
255 branch = namespace.createBranch(256 branch = namespace.createBranch(
256 BranchType.HOSTED, branch_name, requester)257 BranchType.HOSTED, branch_name, requester)
257 except (BranchCreationException, LaunchpadValidationError), e:258 except LaunchpadValidationError, e:
259 msg = e.args[0]
260 if isinstance(msg, unicode):
261 msg = msg.encode('utf-8')
262 return faults.PermissionDenied(msg)
263 except BranchCreationException, e:
258 return faults.PermissionDenied(str(e))264 return faults.PermissionDenied(str(e))
259 else:265 else:
260 return branch.id266 return branch.id
@@ -294,8 +300,7 @@
294 def _serializeControlDirectory(self, requester, product_path,300 def _serializeControlDirectory(self, requester, product_path,
295 trailing_path):301 trailing_path):
296 try:302 try:
297 namespace = lookup_branch_namespace(303 namespace = lookup_branch_namespace(product_path)
298 unescape(product_path).encode('utf-8'))
299 except (InvalidNamespace, NotFoundError):304 except (InvalidNamespace, NotFoundError):
300 return305 return
301 if not ('.bzr' == trailing_path or trailing_path.startswith('.bzr/')):306 if not ('.bzr' == trailing_path or trailing_path.startswith('.bzr/')):
@@ -321,9 +326,9 @@
321 return faults.InvalidPath(path)326 return faults.InvalidPath(path)
322 stripped_path = path.strip('/')327 stripped_path = path.strip('/')
323 for first, second in iter_split(stripped_path, '/'):328 for first, second in iter_split(stripped_path, '/'):
329 first = unescape(first)
324 # Is it a branch?330 # Is it a branch?
325 branch = getUtility(IBranchLookup).getByUniqueName(331 branch = getUtility(IBranchLookup).getByUniqueName(first)
326 unescape(first).encode('utf-8'))
327 if branch is not None:332 if branch is not None:
328 branch = self._serializeBranch(requester, branch, second)333 branch = self._serializeBranch(requester, branch, second)
329 if branch is None:334 if branch is None:
330335
=== modified file 'lib/lp/code/xmlrpc/tests/test_branch.py'
--- lib/lp/code/xmlrpc/tests/test_branch.py 2009-11-12 04:28:28 +0000
+++ lib/lp/code/xmlrpc/tests/test_branch.py 2010-02-22 01:48:21 +0000
@@ -11,6 +11,8 @@
11import unittest11import unittest
12import xmlrpclib12import xmlrpclib
1313
14from bzrlib import urlutils
15
14from zope.security.proxy import removeSecurityProxy16from zope.security.proxy import removeSecurityProxy
1517
16from canonical.config import config18from canonical.config import config
@@ -23,6 +25,9 @@
23from canonical.testing import DatabaseFunctionalLayer25from canonical.testing import DatabaseFunctionalLayer
2426
2527
28NON_ASCII_NAME = u'nam\N{LATIN SMALL LETTER E WITH ACUTE}'
29
30
26class TestExpandURL(TestCaseWithFactory):31class TestExpandURL(TestCaseWithFactory):
27 """Test the way that URLs are expanded."""32 """Test the way that URLs are expanded."""
2833
@@ -54,9 +59,7 @@
54 return branch59 return branch
5560
56 def assertResolves(self, lp_url_path, unique_name):61 def assertResolves(self, lp_url_path, unique_name):
57 """Assert that the given lp URL path expands to the unique name of62 """Assert that `lp_url_path` path expands to `unique_name`."""
58 'branch'.
59 """
60 results = self.api.resolve_lp_path(lp_url_path)63 results = self.api.resolve_lp_path(lp_url_path)
61 # This improves the error message if results happens to be a fault.64 # This improves the error message if results happens to be a fault.
62 if isinstance(results, faults.LaunchpadFault):65 if isinstance(results, faults.LaunchpadFault):
@@ -110,7 +113,7 @@
110 self.assertResolves(113 self.assertResolves(
111 self.product.name, trunk_series.branch.unique_name)114 self.product.name, trunk_series.branch.unique_name)
112115
113 def test_productDoesntExist(self):116 def test_product_doesnt_exist(self):
114 # Return a NoSuchProduct fault if the product doesn't exist.117 # Return a NoSuchProduct fault if the product doesn't exist.
115 self.assertFault(118 self.assertFault(
116 'doesntexist', faults.NoSuchProduct('doesntexist'))119 'doesntexist', faults.NoSuchProduct('doesntexist'))
@@ -141,11 +144,19 @@
141 def test_invalid_product_name(self):144 def test_invalid_product_name(self):
142 # If we get a string that cannot be a name for a product where we145 # If we get a string that cannot be a name for a product where we
143 # expect the name of a product, we should error appropriately.146 # expect the name of a product, we should error appropriately.
144 invalid_name = '+' + self.factory.getUniqueString()147 invalid_name = '_' + self.factory.getUniqueString()
145 self.assertFault(148 self.assertFault(
146 invalid_name,149 invalid_name,
147 faults.InvalidProductIdentifier(invalid_name))150 faults.InvalidProductIdentifier(invalid_name))
148151
152 def test_invalid_product_name_non_ascii(self):
153 # lp:<non-ascii-string> returns InvalidProductIdentifier with the name
154 # escaped.
155 invalid_name = '_' + NON_ASCII_NAME
156 self.assertFault(
157 invalid_name,
158 faults.InvalidProductIdentifier(urlutils.escape(invalid_name)))
159
149 def test_product_and_series(self):160 def test_product_and_series(self):
150 # lp:product/series expands to the branch associated with the product161 # lp:product/series expands to the branch associated with the product
151 # series 'series' on 'product'.162 # series 'series' on 'product'.
@@ -184,6 +195,14 @@
184 '%s/%s' % (self.product.name, "doesntexist"),195 '%s/%s' % (self.product.name, "doesntexist"),
185 faults.NoSuchProductSeries("doesntexist", self.product))196 faults.NoSuchProductSeries("doesntexist", self.product))
186197
198 def test_no_such_product_series_non_ascii(self):
199 # lp:product/<non-ascii-string> returns NoSuchProductSeries with the
200 # name escaped.
201 self.assertFault(
202 '%s/%s' % (self.product.name, NON_ASCII_NAME),
203 faults.NoSuchProductSeries(
204 urlutils.escape(NON_ASCII_NAME), self.product))
205
187 def test_no_such_distro_series(self):206 def test_no_such_distro_series(self):
188 # Return a NoSuchDistroSeries fault if there is no series of the given207 # Return a NoSuchDistroSeries fault if there is no series of the given
189 # name on that distribution.208 # name on that distribution.
@@ -192,6 +211,14 @@
192 '%s/doesntexist/whocares' % distro.name,211 '%s/doesntexist/whocares' % distro.name,
193 faults.NoSuchDistroSeries("doesntexist"))212 faults.NoSuchDistroSeries("doesntexist"))
194213
214 def test_no_such_distro_series_non_ascii(self):
215 # lp:distro/<non-ascii-string>/whatever returns NoSuchDistroSeries
216 # with the name escaped.
217 distro = self.factory.makeDistribution()
218 self.assertFault(
219 '%s/%s/whocares' % (distro.name, NON_ASCII_NAME),
220 faults.NoSuchDistroSeries(urlutils.escape(NON_ASCII_NAME)))
221
195 def test_no_such_source_package(self):222 def test_no_such_source_package(self):
196 # Return a NoSuchSourcePackageName fault if there is no source package223 # Return a NoSuchSourcePackageName fault if there is no source package
197 # of the given name.224 # of the given name.
@@ -201,6 +228,16 @@
201 '%s/%s/doesntexist' % (distribution.name, distroseries.name),228 '%s/%s/doesntexist' % (distribution.name, distroseries.name),
202 faults.NoSuchSourcePackageName('doesntexist'))229 faults.NoSuchSourcePackageName('doesntexist'))
203230
231 def test_no_such_source_package_non_ascii(self):
232 # lp:distro/series/<non-ascii-name> returns NoSuchSourcePackageName
233 # with the name escaped.
234 distroseries = self.factory.makeDistroRelease()
235 distribution = distroseries.distribution
236 self.assertFault(
237 '%s/%s/%s' % (
238 distribution.name, distroseries.name, NON_ASCII_NAME),
239 faults.NoSuchSourcePackageName(urlutils.escape(NON_ASCII_NAME)))
240
204 def test_no_linked_branch_for_source_package(self):241 def test_no_linked_branch_for_source_package(self):
205 # Return a NoLinkedBranch fault if there's no linked branch for the242 # Return a NoLinkedBranch fault if there's no linked branch for the
206 # sourcepackage.243 # sourcepackage.
@@ -234,13 +271,23 @@
234 owner.name, self.product.name)271 owner.name, self.product.name)
235 self.assertResolves(nonexistent_branch, nonexistent_branch)272 self.assertResolves(nonexistent_branch, nonexistent_branch)
236273
274 def test_no_such_branch_product_non_ascii(self):
275 # A path to a branch that contains non ascii characters will never
276 # find a branch, but it still resolves rather than erroring.
277 owner = self.factory.makePerson()
278 nonexistent_branch = u'~%s/%s/%s' % (
279 owner.name, self.product.name, NON_ASCII_NAME)
280 self.assertResolves(
281 nonexistent_branch, urlutils.escape(nonexistent_branch))
282
237 def test_no_such_branch_personal(self):283 def test_no_such_branch_personal(self):
238 # Resolve paths to junk branches.284 # Resolve paths to junk branches.
239 # This test added to make sure we don't raise a fault when looking for285 # This test added to make sure we don't raise a fault when looking for
240 # the '+junk' project, which doesn't actually exist.286 # the '+junk' project, which doesn't actually exist.
241 owner = self.factory.makePerson()287 owner = self.factory.makePerson()
242 nonexistent_branch = '~%s/+junk/doesntexist' % owner.name288 nonexistent_branch = '~%s/+junk/doesntexist' % owner.name
243 self.assertResolves(nonexistent_branch, nonexistent_branch)289 self.assertResolves(
290 nonexistent_branch, urlutils.escape(nonexistent_branch))
244291
245 def test_no_such_branch_package(self):292 def test_no_such_branch_package(self):
246 # Resolve paths to package branches even if there's no branch of that293 # Resolve paths to package branches even if there's no branch of that
@@ -269,6 +316,16 @@
269 nonexistent_owner_branch,316 nonexistent_owner_branch,
270 faults.NoSuchPersonWithName('doesntexist'))317 faults.NoSuchPersonWithName('doesntexist'))
271318
319 def test_resolve_branch_with_no_such_owner_non_ascii(self):
320 # lp:~<non-ascii-string>/product/name returns NoSuchPersonWithName
321 # with the name escaped.
322 nonexistent_owner_branch = u"~%s/%s/%s" % (
323 NON_ASCII_NAME, self.factory.getUniqueString(),
324 self.factory.getUniqueString())
325 self.assertFault(
326 nonexistent_owner_branch,
327 faults.NoSuchPersonWithName(urlutils.escape(NON_ASCII_NAME)))
328
272 def test_too_many_segments(self):329 def test_too_many_segments(self):
273 # If we have more segments than are necessary to refer to a branch,330 # If we have more segments than are necessary to refer to a branch,
274 # then attach these segments to the resolved url.331 # then attach these segments to the resolved url.
275332
=== modified file 'lib/lp/code/xmlrpc/tests/test_codehosting.py'
--- lib/lp/code/xmlrpc/tests/test_codehosting.py 2009-10-19 05:40:52 +0000
+++ lib/lp/code/xmlrpc/tests/test_codehosting.py 2010-02-22 01:48:21 +0000
@@ -676,13 +676,27 @@
676 owner = self.factory.makePerson()676 owner = self.factory.makePerson()
677 product = self.factory.makeProduct()677 product = self.factory.makeProduct()
678 invalid_name = 'invalid name!'678 invalid_name = 'invalid name!'
679 message = ("Invalid branch name %r. %s"679 message = ("Invalid branch name '%s'. %s"
680 % (invalid_name, BRANCH_NAME_VALIDATION_ERROR_MESSAGE))680 % (invalid_name, BRANCH_NAME_VALIDATION_ERROR_MESSAGE))
681 fault = self.branchfs.createBranch(681 fault = self.branchfs.createBranch(
682 owner.id, escape(682 owner.id, escape(
683 '/~%s/%s/%s' % (owner.name, product.name, invalid_name)))683 '/~%s/%s/%s' % (owner.name, product.name, invalid_name)))
684 self.assertFaultEqual(faults.PermissionDenied(message), fault)684 self.assertFaultEqual(faults.PermissionDenied(message), fault)
685685
686 def test_createBranch_unicode_name(self):
687 # Creating a branch with an invalid name fails.
688 owner = self.factory.makePerson()
689 product = self.factory.makeProduct()
690 invalid_name = u'invalid\N{LATIN SMALL LETTER E WITH ACUTE}'
691 message = ("Invalid branch name '%s'. %s"
692 % (invalid_name.encode('utf-8'),
693 str(BRANCH_NAME_VALIDATION_ERROR_MESSAGE)))
694 fault = self.branchfs.createBranch(
695 owner.id, escape(
696 '/~%s/%s/%s' % (owner.name, product.name, invalid_name)))
697 self.assertFaultEqual(
698 faults.PermissionDenied(message), fault)
699
686 def test_createBranch_bad_user(self):700 def test_createBranch_bad_user(self):
687 # Creating a branch under a non-existent user fails.701 # Creating a branch under a non-existent user fails.
688 owner = self.factory.makePerson()702 owner = self.factory.makePerson()
@@ -920,6 +934,13 @@
920 path = '/~%s/%s/no-such-branch' % (requester.name, product.name)934 path = '/~%s/%s/no-such-branch' % (requester.name, product.name)
921 self.assertNotFound(requester, path)935 self.assertNotFound(requester, path)
922936
937 def test_translatePath_no_such_branch_non_ascii(self):
938 requester = self.factory.makePerson()
939 product = self.factory.makeProduct()
940 path = u'/~%s/%s/non-asci\N{LATIN SMALL LETTER I WITH DIAERESIS}' % (
941 requester.name, product.name)
942 self.assertNotFound(requester, escape(path))
943
923 def test_translatePath_private_branch(self):944 def test_translatePath_private_branch(self):
924 requester = self.factory.makePerson()945 requester = self.factory.makePerson()
925 branch = removeSecurityProxy(946 branch = removeSecurityProxy(
926947
=== modified file 'lib/lp/codehosting/inmemory.py'
--- lib/lp/codehosting/inmemory.py 2010-01-06 12:09:46 +0000
+++ lib/lp/codehosting/inmemory.py 2010-02-22 01:48:21 +0000
@@ -555,7 +555,7 @@
555 def createBranch(self, requester_id, branch_path):555 def createBranch(self, requester_id, branch_path):
556 if not branch_path.startswith('/'):556 if not branch_path.startswith('/'):
557 return faults.InvalidPath(branch_path)557 return faults.InvalidPath(branch_path)
558 escaped_path = unescape(branch_path.strip('/')).encode('utf-8')558 escaped_path = unescape(branch_path.strip('/'))
559 try:559 try:
560 namespace_path, branch_name = escaped_path.rsplit('/', 1)560 namespace_path, branch_name = escaped_path.rsplit('/', 1)
561 except ValueError:561 except ValueError:
@@ -565,7 +565,7 @@
565 owner = self._person_set.getByName(data['person'])565 owner = self._person_set.getByName(data['person'])
566 if owner is None:566 if owner is None:
567 return faults.NotFound(567 return faults.NotFound(
568 "User/team %r does not exist." % (data['person'],))568 "User/team '%s' does not exist." % (data['person'],))
569 registrant = self._person_set.get(requester_id)569 registrant = self._person_set.get(requester_id)
570 # The real code consults the branch creation policy of the product. We570 # The real code consults the branch creation policy of the product. We
571 # don't need to do so here, since the tests above this layer never571 # don't need to do so here, since the tests above this layer never
@@ -583,7 +583,7 @@
583 product = self._product_set.getByName(data['product'])583 product = self._product_set.getByName(data['product'])
584 if product is None:584 if product is None:
585 return faults.NotFound(585 return faults.NotFound(
586 "Project %r does not exist." % (data['product'],))586 "Project '%s' does not exist." % (data['product'],))
587 elif data['distribution'] is not None:587 elif data['distribution'] is not None:
588 distro = self._distribution_set.getByName(data['distribution'])588 distro = self._distribution_set.getByName(data['distribution'])
589 if distro is None:589 if distro is None:
@@ -612,7 +612,10 @@
612 sourcepackage=sourcepackage, registrant=registrant,612 sourcepackage=sourcepackage, registrant=registrant,
613 branch_type=BranchType.HOSTED).id613 branch_type=BranchType.HOSTED).id
614 except LaunchpadValidationError, e:614 except LaunchpadValidationError, e:
615 return faults.PermissionDenied(str(e))615 msg = e.args[0]
616 if isinstance(msg, unicode):
617 msg = msg.encode('utf-8')
618 return faults.PermissionDenied(msg)
616619
617 def requestMirror(self, requester_id, branch_id):620 def requestMirror(self, requester_id, branch_id):
618 self._branch_set.get(branch_id).requestMirror()621 self._branch_set.get(branch_id).requestMirror()
619622
=== modified file 'lib/lp/codehosting/vfs/branchfs.py'
--- lib/lp/codehosting/vfs/branchfs.py 2010-01-20 02:15:27 +0000
+++ lib/lp/codehosting/vfs/branchfs.py 2010-02-22 01:48:21 +0000
@@ -612,7 +612,10 @@
612 # parent directories", which is just misleading.612 # parent directories", which is just misleading.
613 fault = trap_fault(613 fault = trap_fault(
614 failure, faults.NotFound, faults.PermissionDenied)614 failure, faults.NotFound, faults.PermissionDenied)
615 raise PermissionDenied(virtual_url_fragment, fault.faultString)615 faultString = fault.faultString
616 if isinstance(faultString, unicode):
617 faultString = faultString.encode('utf-8')
618 raise PermissionDenied(virtual_url_fragment, faultString)
616619
617 return deferred.addErrback(translate_fault)620 return deferred.addErrback(translate_fault)
618621