launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-xmlrpc-check-ref-permissions-not-found into lp:launchpad.

Commit message:
Fix crash if checkRefPermissions finds that the repository is nonexistent.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-xmlrpc-check-ref-permissions-not-found/+merge/365743

This is probably impossible in practice because the repository is only removed from disk some time (currently a week) after it has been deleted from the database, but it's better to be cautious.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-xmlrpc-check-ref-permissions-not-found into lp:launchpad.

=== modified file 'lib/lp/code/xmlrpc/git.py'
--- lib/lp/code/xmlrpc/git.py	2018-12-10 13:54:34 +0000
+++ lib/lp/code/xmlrpc/git.py	2019-04-09 14:30:50 +0000
@@ -348,12 +348,15 @@
             permissions.append('force_push')
         return permissions
 
+    @return_fault
     def _checkRefPermissions(self, requester, translated_path, ref_paths,
                              auth_params):
         if requester == LAUNCHPAD_ANONYMOUS:
             requester = None
         repository = removeSecurityProxy(
             getUtility(IGitLookup).getByHostingPath(translated_path))
+        if repository is None:
+            raise faults.GitRepositoryNotFound(translated_path)
 
         macaroon_raw = auth_params.get("macaroon")
         if (macaroon_raw is not None and
@@ -389,7 +392,7 @@
                 }
 
     def checkRefPermissions(self, translated_path, ref_paths, auth_params):
-        """ See `IGitAPI`"""
+        """See `IGitAPI`."""
         requester_id = auth_params.get("uid")
         if requester_id is None:
             requester_id = LAUNCHPAD_ANONYMOUS

=== modified file 'lib/lp/code/xmlrpc/tests/test_git.py'
--- lib/lp/code/xmlrpc/tests/test_git.py	2018-11-21 00:54:42 +0000
+++ lib/lp/code/xmlrpc/tests/test_git.py	2019-04-09 14:30:50 +0000
@@ -583,6 +583,13 @@
         self.assertHasRefPermissions(
             no_privileges, repository, [path], {path: []})
 
+    def test_checkRefPermissions_nonexistent_repository(self):
+        requester = self.factory.makePerson()
+        self.assertEqual(
+            faults.GitRepositoryNotFound("nonexistent"),
+            self.git_api.checkRefPermissions(
+                "nonexistent", [], {"uid": requester.id}))
+
 
 class TestGitAPI(TestGitAPIMixin, TestCaseWithFactory):
     """Tests for the implementation of `IGitAPI`."""