Allow server handlers to specify capabilities required by clients.

Ideally, the server should be able to handle the absence (as well as
presence) of any capabilities it advertises. However, currently, the
server does not know how to *not* support some capabilities, such
as side-band-64k and delete-refs, which makes it incompatible with
clients that do not request these capabilities. This change adds a
set of "required" capabilites to the server handlers, and causes the
server to fail fast if the client does not request them.

Capabilities should be removed from the required capabilities lists as
their inverses are implemented.

Note that the protocol capabilities spec[1] makes no requirements
about servers supporting the inverse of capabilities.

[1] http://repo.or.cz/w/git.git/blob/HEAD:/Documentation/technical/protocol-capabilities.txt

Change-Id: I6ad2dc3dba67b78b2e195ec0b77eace09a36b0f5

dulwich/server.py

@@ -160,18 +160,13 @@ class Handler(object):
     def capabilities(self):
         raise NotImplementedError(self.capabilities)
 
-<<<<<<< HEAD
-    def set_client_capabilities(self, caps):
-        my_caps = self.capabilities()
-        for cap in caps:
-            if cap not in my_caps:
-                raise GitProtocolError('Client asked for capability %s that '
-                                       'was not advertised.' % cap)
-        self._client_capabilities = set(caps)
-=======
     def innocuous_capabilities(self):
         return ("include-tag", "thin-pack", "no-progress", "ofs-delta")
 
+    def required_capabilities(self):
+        """Return a list of capabilities that we require the client to have."""
+        return []
+
     def set_client_capabilities(self, caps):
         allowable_caps = set(self.innocuous_capabilities())
         allowable_caps.update(self.capabilities())
@@ -179,8 +174,11 @@ class Handler(object):
             if cap not in allowable_caps:
                 raise GitProtocolError('Client asked for capability %s that '
                                        'was not advertised.' % cap)
-        self._client_capabilities = caps
->>>>>>> Refactor server capability code into base Handler.
+        for cap in self.required_capabilities():
+            if cap not in caps:
+                raise GitProtocolError('Client does not support required '
+                                       'capability %s.' % cap)
+        self._client_capabilities = set(caps)
 
     def has_capability(self, cap):
         if self._client_capabilities is None:
@@ -203,14 +201,14 @@ class UploadPackHandler(Handler):
         return ("multi_ack_detailed", "multi_ack", "side-band-64k", "thin-pack",
                 "ofs-delta", "no-progress")
 
-<<<<<<< HEAD
+    def required_capabilities(self):
+        return ("side-band-64k", "thin-pack", "ofs-delta")
+
     def progress(self, message):
         if self.has_capability("no-progress"):
             return
         self.proto.write_sideband(2, message)
 
-=======
->>>>>>> Refactor server capability code into base Handler.
     def handle(self):
         write = lambda x: self.proto.write_sideband(1, x)
 
@@ -532,6 +530,9 @@ class ReceivePackHandler(Handler):
     def capabilities(self):
         return ("report-status", "delete-refs")
 
+    def required_capabilities(self):
+        return ("delete-refs",)
+
     def handle(self):
         refs = self.backend.get_refs().items()
 

dulwich/tests/test_server.py

@@ -81,31 +81,34 @@ class HandlerTestCase(TestCase):
     def setUp(self):
         self._handler = Handler(None, None, None)
         self._handler.capabilities = lambda: ('cap1', 'cap2', 'cap3')
+        self._handler.required_capabilities = lambda: ('cap2',)
 
     def assertSucceeds(self, func, *args, **kwargs):
         try:
             func(*args, **kwargs)
-        except GitProtocolError:
-            self.fail()
+        except GitProtocolError, e:
+            self.fail(e)
 
     def test_capability_line(self):
         self.assertEquals('cap1 cap2 cap3', self._handler.capability_line())
 
     def test_set_client_capabilities(self):
         set_caps = self._handler.set_client_capabilities
-
-        self.assertSucceeds(set_caps, [])
         self.assertSucceeds(set_caps, ['cap2'])
         self.assertSucceeds(set_caps, ['cap1', 'cap2'])
+
         # different order
         self.assertSucceeds(set_caps, ['cap3', 'cap1', 'cap2'])
-        self.assertRaises(GitProtocolError, set_caps, ['capxxx', 'cap1'])
+
+        # error cases
+        self.assertRaises(GitProtocolError, set_caps, ['capxxx', 'cap2'])
+        self.assertRaises(GitProtocolError, set_caps, ['cap1', 'cap3'])
 
         # ignore innocuous but unknown capabilities
-        self.assertRaises(GitProtocolError, set_caps, ['ignoreme'])
+        self.assertRaises(GitProtocolError, set_caps, ['cap2', 'ignoreme'])
         self.assertFalse('ignoreme' in self._handler.capabilities())
         self._handler.innocuous_capabilities = lambda: ('ignoreme',)
-        self.assertSucceeds(set_caps, ['ignoreme'])
+        self.assertSucceeds(set_caps, ['cap2', 'ignoreme'])
 
     def test_has_capability(self):
         self.assertRaises(GitProtocolError, self._handler.has_capability, 'cap')
@@ -122,7 +125,8 @@ class UploadPackHandlerTestCase(TestCase):
         self._handler.proto = TestProto()
 
     def test_progress(self):
-        self._handler.set_client_capabilities([])
+        caps = self._handler.required_capabilities()
+        self._handler.set_client_capabilities(caps)
         self._handler.progress('first message')
         self._handler.progress('second message')
         self.assertEqual('first message',
@@ -132,7 +136,8 @@ class UploadPackHandlerTestCase(TestCase):
         self.assertEqual(None, self._handler.proto.get_received_line(2))
 
     def test_no_progress(self):
-        self._handler.set_client_capabilities(['no-progress'])
+        caps = list(self._handler.required_capabilities()) + ['no-progress']
+        self._handler.set_client_capabilities(caps)
         self._handler.progress('first message')
         self._handler.progress('second message')
         self.assertEqual(None, self._handler.proto.get_received_line(2))