diff mercurial/wireprotoserver.py @ 37062:bbea991635d0

wireproto: service multiple command requests per HTTP request Now that our new frame-based protocol server can understand how to ingest multiple, possibly interleaved, command requests, let's hook it up to the HTTP server. The code on the HTTP side of things is still a bit hacky. We need a bit of work around error handling, content types, etc. But it's a start. Among the added tests, we demonstrate that a client can send frames for multiple commands iterleaved with each other and that a later issued command can respond before the first one has finished sending. This makes our multi-request model technically superior to the previous "batch" command. Differential Revision: https://phab.mercurial-scm.org/D2871
author Gregory Szorc <gregory.szorc@gmail.com>
date Mon, 19 Mar 2018 16:55:07 -0700
parents 2ec1fb9de638
children 884a0c1604ad
line wrap: on
line diff
--- a/mercurial/wireprotoserver.py	Wed Mar 14 16:53:30 2018 -0700
+++ b/mercurial/wireprotoserver.py	Mon Mar 19 16:55:07 2018 -0700
@@ -327,7 +327,12 @@
         _processhttpv2reflectrequest(rctx.repo.ui, rctx.repo, req, res)
         return
 
-    if command not in wireproto.commands:
+    # Extra commands that we handle that aren't really wire protocol
+    # commands. Think extra hard before making this hackery available to
+    # extension.
+    extracommands = {'multirequest'}
+
+    if command not in wireproto.commands and command not in extracommands:
         res.status = b'404 Not Found'
         res.headers[b'Content-Type'] = b'text/plain'
         res.setbodybytes(_('unknown wire protocol command: %s\n') % command)
@@ -338,7 +343,8 @@
 
     proto = httpv2protocolhandler(req, ui)
 
-    if not wireproto.commands.commandavailable(command, proto):
+    if (not wireproto.commands.commandavailable(command, proto)
+        and command not in extracommands):
         res.status = b'404 Not Found'
         res.headers[b'Content-Type'] = b'text/plain'
         res.setbodybytes(_('invalid wire protocol command: %s') % command)
@@ -434,18 +440,14 @@
             # Need more data before we can do anything.
             continue
         elif action == 'runcommand':
-            # We currently only support running a single command per
-            # HTTP request.
-            if seencommand:
-                # TODO define proper error mechanism.
-                res.status = b'200 OK'
-                res.headers[b'Content-Type'] = b'text/plain'
-                res.setbodybytes(_('support for multiple commands per request '
-                                   'not yet implemented'))
+            sentoutput = _httpv2runcommand(ui, repo, req, res, authedperm,
+                                           reqcommand, reactor, meta,
+                                           issubsequent=seencommand)
+
+            if sentoutput:
                 return
 
-            _httpv2runcommand(ui, repo, req, res, authedperm, reqcommand,
-                              reactor, meta)
+            seencommand = True
 
         elif action == 'error':
             # TODO define proper error mechanism.
@@ -471,7 +473,7 @@
                                      % action)
 
 def _httpv2runcommand(ui, repo, req, res, authedperm, reqcommand, reactor,
-                      command):
+                      command, issubsequent):
     """Dispatch a wire protocol command made from HTTPv2 requests.
 
     The authenticated permission (``authedperm``) along with the original
@@ -484,34 +486,57 @@
     # run doesn't have permissions requirements greater than what was granted
     # by ``authedperm``.
     #
-    # For now, this is no big deal, as we only allow a single command per
-    # request and that command must match the command in the URL. But when
-    # things change, we need to watch out...
-    if reqcommand != command['command']:
-        # TODO define proper error mechanism
-        res.status = b'200 OK'
-        res.headers[b'Content-Type'] = b'text/plain'
-        res.setbodybytes(_('command in frame must match command in URL'))
-        return
-
-    # TODO once we get rid of the command==URL restriction, we'll need to
-    # revalidate command validity and auth here. checkperm,
-    # wireproto.commands.commandavailable(), etc.
+    # Our rule for this is we only allow one command per HTTP request and
+    # that command must match the command in the URL. However, we make
+    # an exception for the ``multirequest`` URL. This URL is allowed to
+    # execute multiple commands. We double check permissions of each command
+    # as it is invoked to ensure there is no privilege escalation.
+    # TODO consider allowing multiple commands to regular command URLs
+    # iff each command is the same.
 
     proto = httpv2protocolhandler(req, ui, args=command['args'])
-    assert wireproto.commands.commandavailable(command['command'], proto)
-    wirecommand = wireproto.commands[command['command']]
 
-    assert authedperm in (b'ro', b'rw')
-    assert wirecommand.permission in ('push', 'pull')
+    if reqcommand == b'multirequest':
+        if not wireproto.commands.commandavailable(command['command'], proto):
+            # TODO proper error mechanism
+            res.status = b'200 OK'
+            res.headers[b'Content-Type'] = b'text/plain'
+            res.setbodybytes(_('wire protocol command not available: %s') %
+                             command['command'])
+            return True
+
+        assert authedperm in (b'ro', b'rw')
+        wirecommand = wireproto.commands[command['command']]
+        assert wirecommand.permission in ('push', 'pull')
 
-    # We already checked this as part of the URL==command check, but
-    # permissions are important, so do it again.
-    if authedperm == b'ro':
-        assert wirecommand.permission == 'pull'
-    elif authedperm == b'rw':
-        # We are allowed to access read-only commands under the rw URL.
-        assert wirecommand.permission in ('push', 'pull')
+        if authedperm == b'ro' and wirecommand.permission != 'pull':
+            # TODO proper error mechanism
+            res.status = b'403 Forbidden'
+            res.headers[b'Content-Type'] = b'text/plain'
+            res.setbodybytes(_('insufficient permissions to execute '
+                               'command: %s') % command['command'])
+            return True
+
+        # TODO should we also call checkperm() here? Maybe not if we're going
+        # to overhaul that API. The granted scope from the URL check should
+        # be good enough.
+
+    else:
+        # Don't allow multiple commands outside of ``multirequest`` URL.
+        if issubsequent:
+            # TODO proper error mechanism
+            res.status = b'200 OK'
+            res.headers[b'Content-Type'] = b'text/plain'
+            res.setbodybytes(_('multiple commands cannot be issued to this '
+                               'URL'))
+            return True
+
+        if reqcommand != command['command']:
+            # TODO define proper error mechanism
+            res.status = b'200 OK'
+            res.headers[b'Content-Type'] = b'text/plain'
+            res.setbodybytes(_('command in frame must match command in URL'))
+            return True
 
     rsp = wireproto.dispatch(repo, proto, command['command'])
 
@@ -527,6 +552,7 @@
 
     if action == 'sendframes':
         res.setbodygen(meta['framegen'])
+        return True
     elif action == 'noop':
         pass
     else: