[PATCH 5 of 5 postargs] http: support sending hgargs via POST body instead of in GET or headers
Gregory Szorc
gregory.szorc at gmail.com
Fri Mar 11 13:00:21 EST 2016
On Fri, Mar 11, 2016 at 8:55 AM, Augie Fackler <raf at durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie at google.com>
> # Date 1457714220 18000
> # Fri Mar 11 11:37:00 2016 -0500
> # Node ID 6782893562a73dc56f4184c3967775178c5bdfb4
> # Parent efcc8e438753cddf5af209ae812f77c2affead07
> # EXP-Topic batch
> http: support sending hgargs via POST body instead of in GET or headers
>
> narrowhg (for its narrow spec) and remotefilelog (for its large batch
> requests) would like to be able to make requests with argument sets so
> absurdly large that they blow out total request size limit on some
> http servers. As a workaround, support stuffing args at the start
> of the POST body.
>
> We will probably want to leave this behavior off by default in servers
> forever, because it makes the old "POSTs are only for writes"
> assumption wrong, which might break some of the simpler authentication
> configurations.
>
This patch and series look correct.
I do wish we had a larger discussion about future directions of the
protocol. e.g. can we use the Upgrade header so we can employ WebSockets,
HTTP/2 or a custom protocol that gets us bi-directional streaming, which
would be very beneficial for things like more efficient discovery.
That being said, this is behind an experimental flag and I'm OK with that.
Why stop progress when we have no better alternatives.
> diff --git a/mercurial/hgweb/protocol.py b/mercurial/hgweb/protocol.py
> --- a/mercurial/hgweb/protocol.py
> +++ b/mercurial/hgweb/protocol.py
> @@ -45,6 +45,9 @@ class webproto(wireproto.abstractserverp
> return [data[k] for k in keys]
> def _args(self):
> args = self.req.form.copy()
> + postlen = int(self.req.env.get('HTTP_X_HGARGS_POST', 0))
> + if postlen:
> + return cgi.parse_qs(self.req.read(postlen))
>
Now that data is outside of headers, it feels wasteful to have to URL
encode and turn it into a query string like thing. But, it does mean you
get to reuse the existing parser, so I guess it isn't too bad. If we ever
invent a new, more efficient encoding, we can always invent a new
capability to convey that.
> chunks = []
> i = 1
> while True:
> diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
> --- a/mercurial/httppeer.py
> +++ b/mercurial/httppeer.py
> @@ -97,7 +97,22 @@ class httppeer(wireproto.wirepeer):
> self.ui.debug("sending %s command\n" % cmd)
> q = [('cmd', cmd)]
> headersize = 0
> - if True:
> + # Important: don't use self.capable() here or else you end up
> + # with infinite recursion when trying to look up capabilities
> + # for the first time.
> + postargsok = self.caps is not None and 'httppostargs' in self.caps
> + # TODO: support for httppostargs when data is a file-like
> + # object rather than a basestring
> + canmungedata = not data or isinstance(data, basestring)
> + if postargsok and canmungedata:
> + strargs = urllib.urlencode(sorted(args.items()))
> + if strargs:
> + if not data:
> + data = strargs
> + elif isinstance(data, basestring):
> + data = strargs + data
> + headers['X-HgArgs-Post'] = len(strargs)
> + else:
> if len(args) > 0:
> httpheader = self.capable('httpheader')
> if httpheader:
> diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
> --- a/mercurial/wireproto.py
> +++ b/mercurial/wireproto.py
> @@ -688,6 +688,8 @@ def _capabilities(repo, proto):
> caps.append('unbundle=%s' % ','.join(changegroupmod.bundlepriority))
> caps.append(
> 'httpheader=%d' % repo.ui.configint('server', 'maxhttpheaderlen',
> 1024))
> + if repo.ui.configbool('experimental', 'httppostargs', False):
> + caps.append('httppostargs')
> return caps
>
> # If you are writing an extension and consider wrapping this function.
> Wrap
> diff --git a/tests/test-wireproto.t b/tests/test-wireproto.t
> --- a/tests/test-wireproto.t
> +++ b/tests/test-wireproto.t
> @@ -19,7 +19,12 @@ Local:
>
> HTTP:
>
> - $ hg serve -R repo -p $HGPORT -d --pid-file=hg1.pid -E error.log -A
> access.log
> + $ cat >> $HGRCPATH <<EOF
> + > [experimental]
> + > httppostargs = true
> + > EOF
> +
> + $ hg serve -R repo -p $HGPORT -d --pid-file=hg1.pid -E error.log -A
> access.log
> $ cat hg1.pid >> $DAEMON_PIDS
>
> $ hg debugwireargs http://localhost:$HGPORT/ un deux trois quatre
> @@ -37,6 +42,66 @@ HTTP:
> $ cat error.log
> $ cat access.log
> * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:39
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:39
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:43
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:43
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:27
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:27
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:1033
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:1033
> (glob)
> +
> + $ cat >> $HGRCPATH <<EOF
> + > [experimental]
> + > httppostargs = false
> + > EOF
> +
> +HTTP without args-in-POST:
> + $ hg serve -R repo -p $HGPORT1 -d --pid-file=hg1.pid -E error.log -A
> access.log
> + $ cat hg1.pid >> $DAEMON_PIDS
> +
> + $ hg debugwireargs http://localhost:$HGPORT1/ un deux trois quatre
> + un deux trois quatre None
> + $ hg debugwireargs http://localhost:$HGPORT1/ \ un deux trois\ qu\ \
> atre
> + un deux trois qu atre None
> + $ hg debugwireargs http://localhost:$HGPORT1/ eins zwei --four vier
> + eins zwei None vier None
> + $ hg debugwireargs http://localhost:$HGPORT1/ eins zwei
> + eins zwei None None None
> + $ hg debugwireargs http://localhost:$HGPORT1/ eins zwei --five fuenf
> + eins zwei None None None
> + $ hg debugwireargs http://localhost:$HGPORT1/ un deux trois
> onethousandcharactersxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> + un deux trois
> onethousandcharactersxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
> None
> + $ cat error.log
> + $ cat access.log
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:39
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:39
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:43
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:43
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:27
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:27
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:17
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:1033
> (glob)
> + * - - [*] "POST /?cmd=debugwireargs HTTP/1.1" 200 - x-hgargs-post:1033
> (glob)
> + * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> * - - [*] "GET /?cmd=debugwireargs HTTP/1.1" 200 -
> x-hgarg-1:four=quatre&one=un&three=trois&two=deux (glob)
> * - - [*] "GET /?cmd=debugwireargs HTTP/1.1" 200 -
> x-hgarg-1:four=quatre&one=un&three=trois&two=deux (glob)
> * - - [*] "GET /?cmd=capabilities HTTP/1.1" 200 - (glob)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160311/57e94278/attachment.html>
More information about the Mercurial-devel
mailing list