D2837: wireproto: require POST for all HTTPv2 requests
indygreg (Gregory Szorc)
phabricator at mercurial-scm.org
Tue Mar 13 02:16:44 UTC 2018
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
Wire protocol version 1 transfers argument data via request
headers by default. This has historically caused problems because
servers institute limits on the length of individual HTTP headers
as well as the total size of all request headers. Mercurial servers
can advertise the maximum length of an individual header. But
there's no guarantee any intermediate HTTP agents will accept
headers up to that length.
In the existing wire protocol, server operators typically also
key off the HTTP request method to implement authentication.
For example, GET requests translate to read-only requests and
can be allowed. But read-write commands must use POST and require
authentication. This has typically worked because the only wire
protocol commands that use POST modify the repo (e.g. the
"unbundle" command).
There is an experimental feature to enable clients to transmit
argument data via POST request bodies. This is technically a
better and more robust solution. But we can't enable it by default
because of servers assuming POST means write access.
In version 2 of the wire protocol, the permissions of a request
are encoded in the URL. And with it being a new protocol in a new
URL space, we're not constrained by backwards compatibility
requirements.
This commit adopts the technically superior mechanism of using
HTTP request bodies to send argument data by requiring POST for
all commands. Strictly speaking, it may be possible to send
request bodies on GET requests. But my experience is that not all
HTTP stacks support this. POST pretty much always works. Using POST
for read-only operations does sacrifice some RESTful design
purity. But this API cares about practicality, not about being
in Roy T. Fielding's REST ivory tower.
There's a chance we may relax this restriction in the future. But
for now, I want to see how far we can get with a POST only API.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2837
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
@@ -32,41 +32,53 @@
Request to read-only command works out of the box
- $ get-with-headers.py $LOCALIP:$HGPORT api/$HTTPV2/ro/known -
+ $ get-with-headers.py --method POST $LOCALIP:$HGPORT api/$HTTPV2/ro/known -
200 OK
content-length: 9
content-type: text/plain
ro/known/ (no-eol)
Request to unknown command yields 404
- $ get-with-headers.py $LOCALIP:$HGPORT api/$HTTPV2/ro/badcommand -
+ $ get-with-headers.py --method POST $LOCALIP:$HGPORT api/$HTTPV2/ro/badcommand -
404 Not Found
content-length: 42
content-type: text/plain
unknown wire protocol command: badcommand
[1]
+Only POST is allowed
+
+ $ get-with-headers.py --method GET $LOCALIP:$HGPORT api/$HTTPV2/ro/known -
+ 405 Method Not Allowed
+ allow: POST
+ content-length: 30
+
+ commands require POST requests (no-eol)
+ [1]
+
Request to read-write command fails because server is read-only by default
GET request not allowed
$ get-with-headers.py $LOCALIP:$HGPORT api/$HTTPV2/rw/known -
- 405 push requires POST request
- content-length: 17
+ 405 Method Not Allowed
+ allow: POST
+ content-length: 30
- permission denied (no-eol)
+ commands require POST requests (no-eol)
[1]
Even for unknown commands
$ get-with-headers.py $LOCALIP:$HGPORT api/$HTTPV2/rw/badcommand -
- 405 push requires POST request
- content-length: 17
+ 405 Method Not Allowed
+ allow: POST
+ content-length: 30
- permission denied (no-eol)
+ commands require POST requests (no-eol)
[1]
diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -299,6 +299,12 @@
res.setbodybytes(_('unknown permission: %s') % permission)
return
+ if req.method != 'POST':
+ res.status = b'405 Method Not Allowed'
+ res.headers[b'Allow'] = b'POST'
+ res.setbodybytes(_('commands require POST requests'))
+ return
+
# At some point we'll want to use our own API instead of recycling the
# behavior of version 1 of the wire protocol...
# TODO return reasonable responses - not responses that overload the
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
@@ -152,11 +152,14 @@
Version 2 of the HTTP protocol is exposed under the ``/api/*`` URL space.
It's final API name is not yet formalized.
-Commands are triggered by sending HTTP requests against URLs of the
+Commands are triggered by sending HTTP POST requests against URLs of the
form ``<permission>/<command>``, where ``<permission>`` is ``ro`` or
``rw``, meaning read-only and read-write, respectively and ``<command>``
is a named wire protocol command.
+Non-POST request methods MUST be rejected by the server with an HTTP
+405 response.
+
Commands that modify repository state in meaningful ways MUST NOT be
exposed under the ``ro`` URL prefix. All available commands MUST be
available under the ``rw`` URL prefix.
To: indygreg, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list