[PATCH] httprepo: long arguments support (issue2126)

Matt Mackall mpm at selenic.com
Sun Mar 20 10:27:13 CDT 2011


On Sun, 2011-03-20 at 18:10 +0800, Steven Brown wrote:
> # HG changeset patch
> # User Steven Brown <StevenGBrown at gmail.com>
> # Date 1300614413 -28800
> # Node ID dd1002a1476d86eace9724ef04b502b724fc0525
> # Parent  48d606d7192b6f8f4fc8071fccdc5a83843ab2d3
> httprepo: long arguments support (issue2126)
> 
> Send the command arguments in the HTTP request body. The command is still part
> of the URL. If the server does not have the 'longargs' capability, the client
> will send the command arguments in the URL as it did previously.
> 
> When sending both command arguments and additional data in the same request,
> the client adds a 'datastart' field to the URL. The server uses this to
> determine where the command arguments end and the additional data begins.

One of the reasons we were focusing on stashing stuff in headers is that
some folks might be relying on non-push requests being GETs in their web
server access rules. I'm not sure if this is a real problem, though.
Obviously, a POST approach is a lot cleaner.

> Rejected approaches:
> - Send the command arguments in the HTTP request header. This doesn't help
>   much, because Apache by default has the same limit on URL size as it does
>   for each header.

I think the limit on each cookie is finite, but that there can be
multiple cookies?

> Potential follow-ups:
> - httprepo._callstream: If the command arguments are small enough, it may be
>   better to send them directly and avoid creation of a temporary file.
> - Some commands currently call the server several times, but would only need
>   to make one call with the long arguments support. For example, see the
>   wireproto.between function and the retrieval of branches by the
>   discovery.findcommonincoming function.
> - Allow clients to send batches of server commands in one roundtrip.
> 
> diff -r 48d606d7192b -r dd1002a1476d mercurial/hgweb/protocol.py
> --- a/mercurial/hgweb/protocol.py	Sun Mar 20 01:16:57 2011 +0100
> +++ b/mercurial/hgweb/protocol.py	Sun Mar 20 17:46:53 2011 +0800
> @@ -5,7 +5,7 @@
>  # This software may be used and distributed according to the terms of the
>  # GNU General Public License version 2 or any later version.
>  
> -import cStringIO, zlib, sys, urllib
> +import cgi, cStringIO, zlib, sys, urllib
>  from mercurial import util, wireproto
>  from common import HTTP_OK
>  
> @@ -15,7 +15,14 @@
>      def __init__(self, req):
>          self.req = req
>          self.response = ''
> +        self.datastart = 0
>      def getargs(self, args):
> +        if 'datastart' in self.req.form.keys():
> +            self.datastart = int(self.req.form['datastart'][0])
> +            argsdata = ''
> +            for s in util.filechunkiter(self.req, limit=self.datastart):
> +                argsdata += s
> +            self.req.form = cgi.parse_qs(argsdata)
>          data = {}
>          keys = args.split()
>          for k in keys:
> @@ -29,7 +36,7 @@
>                  data[k] = self.req.form[k][0]
>          return [data[k] for k in keys]
>      def getfile(self, fp):
> -        length = int(self.req.env['CONTENT_LENGTH'])
> +        length = int(self.req.env['CONTENT_LENGTH']) - self.datastart
>          for s in util.filechunkiter(self.req, limit=length):
>              fp.write(s)
>      def redirect(self):
> diff -r 48d606d7192b -r dd1002a1476d mercurial/httprepo.py
> --- a/mercurial/httprepo.py	Sun Mar 20 01:16:57 2011 +0100
> +++ b/mercurial/httprepo.py	Sun Mar 20 17:46:53 2011 +0800
> @@ -9,7 +9,7 @@
>  from node import nullid
>  from i18n import _
>  import changegroup, statichttprepo, error, url, util, wireproto
> -import os, urllib, urllib2, urlparse, zlib, httplib
> +import tempfile, os, shutil, urllib, urllib2, urlparse, zlib, httplib
>  import errno, socket
>  
>  def zgenerator(f):
> @@ -72,18 +72,37 @@
>  
>      def _callstream(self, cmd, **args):
>          if cmd == 'pushkey':
> -            args['data'] = ''
> +            args['data'] = None
>          data = args.pop('data', None)
>          headers = args.pop('headers', {})
>          self.ui.debug("sending %s command\n" % cmd)
> -        q = [('cmd', cmd)] + sorted(args.items())
> +        q = [('cmd', cmd)]
> +        reqbody = None
> +        if len(args) > 0 and self.capable('longargs'):
> +            fh = None
> +            filename = None
> +            try:
> +                fd, filename = tempfile.mkstemp(prefix="args")
> +                fh = os.fdopen(fd, "wb")
> +                encodedargs = urllib.urlencode(sorted(args.items()))
> +                fh.write(encodedargs)
> +                if data is not None:
> +                    q += [('datastart', len(encodedargs))]
> +                    shutil.copyfileobj(data, fh)
> +            finally:
> +                if fh is not None:
> +                    fh.close()
> +            reqbody = url.httpsendfile(self.ui, filename, "rb")
> +        else:
> +            q += sorted(args.items())
> +            reqbody = data
>          qs = '?%s' % urllib.urlencode(q)
>          cu = "%s%s" % (self._url, qs)
> -        req = urllib2.Request(cu, data, headers)
> -        if data is not None:
> -            # len(data) is broken if data doesn't fit into Py_ssize_t
> +        req = urllib2.Request(cu, reqbody, headers)
> +        if reqbody is not None:
> +            # len(reqbody) is broken if reqbody doesn't fit into Py_ssize_t
>              # add the header ourself to avoid OverflowError
> -            size = data.__len__()
> +            size = reqbody.__len__()
>              self.ui.debug("sending %s bytes\n" % size)
>              req.add_unredirected_header('Content-Length', '%d' % size)
>          try:
> diff -r 48d606d7192b -r dd1002a1476d mercurial/wireproto.py
> --- a/mercurial/wireproto.py	Sun Mar 20 01:16:57 2011 +0100
> +++ b/mercurial/wireproto.py	Sun Mar 20 17:46:53 2011 +0800
> @@ -176,7 +176,7 @@
>      return "".join(r)
>  
>  def capabilities(repo, proto):
> -    caps = 'lookup changegroupsubset branchmap pushkey'.split()
> +    caps = 'lookup changegroupsubset branchmap pushkey longargs'.split()
>      if _allowstream(repo.ui):
>          requiredformats = repo.requirements & repo.supportedformats
>          # if our local revlogs are just revlogv1, add 'stream' cap
> diff -r 48d606d7192b -r dd1002a1476d tests/test-hgweb-commands.t
> --- a/tests/test-hgweb-commands.t	Sun Mar 20 01:16:57 2011 +0100
> +++ b/tests/test-hgweb-commands.t	Sun Mar 20 17:46:53 2011 +0800
> @@ -905,7 +905,7 @@
>    $ "$TESTDIR/get-with-headers.py" 127.0.0.1:$HGPORT '?cmd=capabilities'; echo
>    200 Script output follows
>    
> -  lookup changegroupsubset branchmap pushkey unbundle=HG10GZ,HG10BZ,HG10UN
> +  lookup changegroupsubset branchmap pushkey longargs unbundle=HG10GZ,HG10BZ,HG10UN
>  
>  heads
>  
> diff -r 48d606d7192b -r dd1002a1476d tests/test-http-proxy.t
> --- a/tests/test-http-proxy.t	Sun Mar 20 01:16:57 2011 +0100
> +++ b/tests/test-http-proxy.t	Sun Mar 20 17:46:53 2011 +0800
> @@ -100,21 +100,21 @@
>    $ cat proxy.log
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=capabilities HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=stream_out HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=listkeys&namespace=bookmarks HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=listkeys HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=capabilities HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=heads HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=changegroup&roots=0000000000000000000000000000000000000000 HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=listkeys&namespace=bookmarks HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=changegroup HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=listkeys HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=capabilities HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=heads HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=changegroup&roots=0000000000000000000000000000000000000000 HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=listkeys&namespace=bookmarks HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=changegroup HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=listkeys HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=capabilities HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=heads HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=changegroup&roots=0000000000000000000000000000000000000000 HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=listkeys&namespace=bookmarks HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=changegroup HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=listkeys HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=capabilities HTTP/1.1" - - (glob)
>    * - - [*] "GET http://localhost:$HGPORT/?cmd=heads HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=changegroup&roots=0000000000000000000000000000000000000000 HTTP/1.1" - - (glob)
> -  * - - [*] "GET http://localhost:$HGPORT/?cmd=listkeys&namespace=bookmarks HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=changegroup HTTP/1.1" - - (glob)
> +  * - - [*] "POST http://localhost:$HGPORT/?cmd=listkeys HTTP/1.1" - - (glob)
>  
> diff -r 48d606d7192b -r dd1002a1476d tests/test-push-http.t
> --- a/tests/test-push-http.t	Sun Mar 20 01:16:57 2011 +0100
> +++ b/tests/test-push-http.t	Sun Mar 20 17:46:53 2011 +0800
> @@ -66,6 +66,32 @@
>    repository tip rolled back to revision 0 (undo serve)
>    working directory now based on revision 0
>  
> +expect success, server lacks the longargs capability
> +
> +  $ cat > longargs-off.py << EOF
> +  > from mercurial import extensions, repo
> +  > def extsetup():
> +  >     extensions.wrapfunction(repo.repository, 'capable', wrapper)
> +  > def wrapper(orig, self, name, *args, **kwargs):
> +  >     if name == 'longargs':
> +  >         return False
> +  >     return orig(self, name, *args, **kwargs)
> +  > EOF
> +  $ echo '[extensions]' >> .hg/hgrc
> +  $ echo "longargs-off = `pwd`/longargs-off.py" >> .hg/hgrc
> +  $ req
> +  pushing to http://localhost:$HGPORT/
> +  searching for changes
> +  remote: adding changesets
> +  remote: adding manifests
> +  remote: adding file changes
> +  remote: added 1 changesets with 1 changes to 1 files
> +  remote: changegroup hook: HG_NODE=ba677d0156c1196c1a699fa53f390dcfc3ce3872 HG_SOURCE=serve HG_URL=remote:http:127.0.0.1: 
> +  % serve errors
> +  $ hg rollback
> +  repository tip rolled back to revision 0 (undo serve)
> +  working directory now based on revision 0
> +
>  expect authorization error: all users denied
>  
>    $ echo '[web]' > .hg/hgrc
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list