D2871: wireproto: service multiple command requests per HTTP request
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Thu Mar 15 01:11:34 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
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.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2871
AFFECTED FILES
mercurial/help/internals/wireprotocol.txt
mercurial/wireprotoserver.py
tests/test-http-api-httpv2.t
CHANGE DETAILS
diff --git a/tests/test-http-api-httpv2.t b/tests/test-http-api-httpv2.t
--- a/tests/test-http-api-httpv2.t
+++ b/tests/test-http-api-httpv2.t
@@ -397,4 +397,153 @@
s> received: <no frame>\n
s> {"action": "noop"}
+Multiple requests to regular command URL are not allowed
+
+ $ send << EOF
+ > httprequest POST api/$HTTPV2/ro/capabilities
+ > accept: $MEDIATYPE
+ > content-type: $MEDIATYPE
+ > user-agent: test
+ > frame 1 command-name eos capabilities
+ > frame 3 command-name eos capabilities
+ > EOF
+ using raw connection to peer
+ s> POST /api/exp-http-v2-0001/ro/capabilities HTTP/1.1\r\n
+ s> Accept-Encoding: identity\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
+ s> content-type: application/mercurial-exp-framing-0002\r\n
+ s> user-agent: test\r\n
+ s> content-length: 36\r\n
+ s> host: $LOCALIP:$HGPORT\r\n (glob)
+ s> \r\n
+ s> \x0c\x00\x00\x01\x00\x11capabilities\x0c\x00\x00\x03\x00\x11capabilities
+ s> makefile('rb', None)
+ s> HTTP/1.1 200 OK\r\n
+ s> Server: testing stub value\r\n
+ s> Date: $HTTP_DATE$\r\n
+ s> Content-Type: text/plain\r\n
+ s> Content-Length: 46\r\n
+ s> \r\n
+ s> multiple commands cannot be issued to this URL
+
+Multiple requests to "multirequest" URL are allowed
+
+ $ send << EOF
+ > httprequest POST api/$HTTPV2/ro/multirequest
+ > accept: $MEDIATYPE
+ > content-type: $MEDIATYPE
+ > user-agent: test
+ > frame 1 command-name eos capabilities
+ > frame 3 command-name eos capabilities
+ > EOF
+ using raw connection to peer
+ s> POST /api/exp-http-v2-0001/ro/multirequest HTTP/1.1\r\n
+ s> Accept-Encoding: identity\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
+ s> content-type: application/mercurial-exp-framing-0002\r\n
+ s> user-agent: test\r\n
+ s> content-length: 36\r\n
+ s> host: $LOCALIP:$HGPORT\r\n (glob)
+ s> \r\n
+ s> \x0c\x00\x00\x01\x00\x11capabilities\x0c\x00\x00\x03\x00\x11capabilities
+ s> makefile('rb', None)
+ s> HTTP/1.1 200 OK\r\n
+ s> Server: testing stub value\r\n
+ s> Date: $HTTP_DATE$\r\n
+ s> Content-Type: application/mercurial-exp-framing-0002\r\n
+ s> Transfer-Encoding: chunked\r\n
+ s> \r\n
+ s> *\r\n (glob)
+ s> Y\x01\x00\x01\x00Blookup branchmap pushkey known getbundle unbundlehash streamreqs=generaldelta,revlogv1 $USUAL_BUNDLE2_CAPS_SERVER$ unbundle=HG10GZ,HG10BZ,HG10UN
+ s> \r\n
+ s> *\r\n (glob)
+ s> Y\x01\x00\x03\x00Blookup branchmap pushkey known getbundle unbundlehash streamreqs=generaldelta,revlogv1 $USUAL_BUNDLE2_CAPS_SERVER$ unbundle=HG10GZ,HG10BZ,HG10UN
+ s> \r\n
+ s> 0\r\n
+ s> \r\n
+
+Interleaved requests to "multirequest" are processed
+
+ $ send << EOF
+ > httprequest POST api/$HTTPV2/ro/multirequest
+ > accept: $MEDIATYPE
+ > content-type: $MEDIATYPE
+ > user-agent: test
+ > frame 1 command-name have-args listkeys
+ > frame 3 command-name have-args listkeys
+ > frame 3 command-argument eoa \x09\x00\x09\x00namespacebookmarks
+ > frame 1 command-argument eoa \x09\x00\x0a\x00namespacenamespaces
+ > EOF
+ using raw connection to peer
+ s> POST /api/exp-http-v2-0001/ro/multirequest HTTP/1.1\r\n
+ s> Accept-Encoding: identity\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
+ s> content-type: application/mercurial-exp-framing-0002\r\n
+ s> user-agent: test\r\n
+ s> content-length: 85\r\n
+ s> host: $LOCALIP:$HGPORT\r\n (glob)
+ s> \r\n
+ s> \x08\x00\x00\x01\x00\x12listkeys\x08\x00\x00\x03\x00\x12listkeys\x16\x00\x00\x03\x00" \x00 \x00namespacebookmarks\x17\x00\x00\x01\x00" \x00\n
+ s> \x00namespacenamespaces
+ s> makefile('rb', None)
+ s> HTTP/1.1 200 OK\r\n
+ s> Server: testing stub value\r\n
+ s> Date: $HTTP_DATE$\r\n
+ s> Content-Type: application/mercurial-exp-framing-0002\r\n
+ s> Transfer-Encoding: chunked\r\n
+ s> \r\n
+ s> 6\r\n
+ s> \x00\x00\x00\x03\x00B
+ s> \r\n
+ s> 24\r\n
+ s> \x1e\x00\x00\x01\x00Bbookmarks \n
+ s> namespaces \n
+ s> phases
+ s> \r\n
+ s> 0\r\n
+ s> \r\n
+
+Restart server to disable read-write access
+
+ $ killdaemons.py
+ $ cat > server/.hg/hgrc << EOF
+ > [experimental]
+ > web.apiserver = true
+ > web.api.debugreflect = true
+ > web.api.http-v2 = true
+ > [web]
+ > push_ssl = false
+ > EOF
+
+ $ hg -R server serve -p $HGPORT -d --pid-file hg.pid -E error.log
+ $ cat hg.pid > $DAEMON_PIDS
+
+Attempting to run a read-write command via multirequest on read-only URL is not allowed
+
+ $ send << EOF
+ > httprequest POST api/$HTTPV2/ro/multirequest
+ > accept: $MEDIATYPE
+ > content-type: $MEDIATYPE
+ > user-agent: test
+ > frame 1 command-name eos unbundle
+ > EOF
+ using raw connection to peer
+ s> POST /api/exp-http-v2-0001/ro/multirequest HTTP/1.1\r\n
+ s> Accept-Encoding: identity\r\n
+ s> accept: application/mercurial-exp-framing-0002\r\n
+ s> content-type: application/mercurial-exp-framing-0002\r\n
+ s> user-agent: test\r\n
+ s> content-length: 14\r\n
+ s> host: $LOCALIP:$HGPORT\r\n (glob)
+ s> \r\n
+ s> \x08\x00\x00\x01\x00\x11unbundle
+ s> makefile('rb', None)
+ s> HTTP/1.1 403 Forbidden\r\n
+ s> Server: testing stub value\r\n
+ s> Date: $HTTP_DATE$\r\n
+ s> Content-Type: text/plain\r\n
+ s> Content-Length: 53\r\n
+ s> \r\n
+ s> insufficient permissions to execute command: unbundle
+
$ cat error.log
diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -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:
diff --git a/mercurial/help/internals/wireprotocol.txt b/mercurial/help/internals/wireprotocol.txt
--- a/mercurial/help/internals/wireprotocol.txt
+++ b/mercurial/help/internals/wireprotocol.txt
@@ -203,6 +203,28 @@
Servers receiving requests with an invalid ``Content-Type`` header SHOULD
respond with an HTTP 415.
+The command to run is specified in the POST payload as defined by the
+*Unified Frame-Based Protocol*. This is redundant with data already
+encoded in the URL. This is by design, so server operators can have
+better understanding about server activity from looking merely at
+HTTP access logs.
+
+In most circumstances, the command specified in the URL MUST match
+the command specified in the frame-based payload or the server will
+respond with an error. The exception to this is the special
+``multirequest`` URL. (See below.) In addition, HTTP requests
+are limited to one command invocation. The exception is the special
+``multirequest`` URL.
+
+The ``multirequest`` command endpoints (``ro/multirequest`` and
+``rw/multirequest``) are special in that they allow the execution of
+*any* command and allow the execution of multiple commands. If the
+HTTP request issues multiple commands across multiple frames, all
+issued commands will be processed by the server. Per the defined
+behavior of the *Unified Frame-Based Protocol*, commands may be
+issued interleaved and responses may come back in a different order
+than they were issued. Clients MUST be able to deal with this.
+
SSH Protocol
============
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list