D2837: wireproto: require POST for all HTTPv2 requests

[indygreg (Gregory Szorc)]

indygreg updated this revision to Diff 7136.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2837?vs=7015&id=7136

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
@@ -63,11 +63,11 @@
 Request to read-only command works out of the box
 
   $ send << EOF
-  > httprequest GET api/$HTTPV2/ro/customreadonly
+  > httprequest POST api/$HTTPV2/ro/customreadonly
   >     user-agent: test
   > EOF
   using raw connection to peer
-  s>     GET /api/exp-http-v2-0001/ro/customreadonly HTTP/1.1\r\n
+  s>     POST /api/exp-http-v2-0001/ro/customreadonly HTTP/1.1\r\n
   s>     Accept-Encoding: identity\r\n
   s>     user-agent: test\r\n
   s>     host: $LOCALIP:$HGPORT\r\n (glob)
@@ -84,11 +84,11 @@
 Request to unknown command yields 404
 
   $ send << EOF
-  > httprequest GET api/$HTTPV2/ro/badcommand
+  > httprequest POST api/$HTTPV2/ro/badcommand
   >     user-agent: test
   > EOF
   using raw connection to peer
-  s>     GET /api/exp-http-v2-0001/ro/badcommand HTTP/1.1\r\n
+  s>     POST /api/exp-http-v2-0001/ro/badcommand HTTP/1.1\r\n
   s>     Accept-Encoding: identity\r\n
   s>     user-agent: test\r\n
   s>     host: $LOCALIP:$HGPORT\r\n (glob)
@@ -102,9 +102,30 @@
   s>     \r\n
   s>     unknown wire protocol command: badcommand\n
 
+GET to read-only command yields a 405
+
+  $ send << EOF
+  > httprequest GET api/$HTTPV2/ro/customreadonly
+  >     user-agent: test
+  > EOF
+  using raw connection to peer
+  s>     GET /api/exp-http-v2-0001/ro/customreadonly HTTP/1.1\r\n
+  s>     Accept-Encoding: identity\r\n
+  s>     user-agent: test\r\n
+  s>     host: $LOCALIP:$HGPORT\r\n (glob)
+  s>     \r\n
+  s> makefile('rb', None)
+  s>     HTTP/1.1 405 Method Not Allowed\r\n
+  s>     Server: testing stub value\r\n
+  s>     Date: $HTTP_DATE$\r\n
+  s>     Allow: POST\r\n
+  s>     Content-Length: 30\r\n
+  s>     \r\n
+  s>     commands require POST requests
+
 Request to read-write command fails because server is read-only by default
 
-GET to read-write request not allowed
+GET to read-write request yields 405
 
   $ send << EOF
   > httprequest GET api/$HTTPV2/rw/customreadonly
@@ -117,12 +138,13 @@
   s>     host: $LOCALIP:$HGPORT\r\n (glob)
   s>     \r\n
   s> makefile('rb', None)
-  s>     HTTP/1.1 405 push requires POST request\r\n
+  s>     HTTP/1.1 405 Method Not Allowed\r\n
   s>     Server: testing stub value\r\n
   s>     Date: $HTTP_DATE$\r\n
-  s>     Content-Length: 17\r\n
+  s>     Allow: POST\r\n
+  s>     Content-Length: 30\r\n
   s>     \r\n
-  s>     permission denied
+  s>     commands require POST requests
 
 Even for unknown commands
 
@@ -137,12 +159,13 @@
   s>     host: $LOCALIP:$HGPORT\r\n (glob)
   s>     \r\n
   s> makefile('rb', None)
-  s>     HTTP/1.1 405 push requires POST request\r\n
+  s>     HTTP/1.1 405 Method Not Allowed\r\n
   s>     Server: testing stub value\r\n
   s>     Date: $HTTP_DATE$\r\n
-  s>     Content-Length: 17\r\n
+  s>     Allow: POST\r\n
+  s>     Content-Length: 30\r\n
   s>     \r\n
-  s>     permission denied
+  s>     commands require POST requests
 
 SSL required by default
 
@@ -173,38 +196,6 @@
   > web.api.http-v2 = true
   > [web]
   > push_ssl = false
-  > EOF
-
-  $ hg -R server serve -p $HGPORT -d --pid-file hg.pid
-  $ cat hg.pid > $DAEMON_PIDS
-
-Server insists on POST for read-write commands
-
-  $ send << EOF
-  > httprequest GET api/$HTTPV2/rw/customreadonly
-  >     user-agent: test
-  > EOF
-  using raw connection to peer
-  s>     GET /api/exp-http-v2-0001/rw/customreadonly HTTP/1.1\r\n
-  s>     Accept-Encoding: identity\r\n
-  s>     user-agent: test\r\n
-  s>     host: $LOCALIP:$HGPORT\r\n (glob)
-  s>     \r\n
-  s> makefile('rb', None)
-  s>     HTTP/1.1 405 push requires POST request\r\n
-  s>     Server: testing stub value\r\n
-  s>     Date: $HTTP_DATE$\r\n
-  s>     Content-Length: 17\r\n
-  s>     \r\n
-  s>     permission denied
-
-  $ killdaemons.py
-  $ cat > server/.hg/hgrc << EOF
-  > [experimental]
-  > web.apiserver = true
-  > web.api.http-v2 = true
-  > [web]
-  > push_ssl = false
   > allow-push = *
   > EOF
 
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