[PATCH 5 of 5 postargs] http: support sending hgargs via POST body instead of in GET or headers
Martin von Zweigbergk
martinvonz at google.com
Fri Mar 11 13:02:48 EST 2016
On Fri, Mar 11, 2016 at 10:00 AM, Gregory Szorc <gregory.szorc at gmail.com> wrote:
> 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.
I haven't reviewed the patch yet, but I did suggest
pushkey.{encode,decode}keys(). Would that be better?
>
>>
>> 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
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
More information about the Mercurial-devel
mailing list