[PATCH 1 of 1] hgwebdir_mod.py can now handle 'baseurl' configurations with leading slash

[Matt Mackall]

On Mon, 2011-08-01 at 20:24 +0200, Wujek Srujek wrote:
> Hi,
> 
> On Mon, Aug 1, 2011 at 8:10 PM, Matt Mackall <mpm at selenic.com> wrote:
> 
> > On Mon, 2011-08-01 at 12:03 +0200, wujek.srujek at googlemail.com wrote:
> > > # HG changeset patch
> > > # User wujek
> > > # Date 1312184890 -7200
> > > # Node ID 060a118fc62277c5cd71751503db19a7b4183a21
> > > # Parent  cc2c22511707b13d045f17bb34e865d2393c53cf
> > > hgwebdir_mod.py can now handle 'baseurl' configurations with leading
> > slash
> >
> > I've queued the fix for stable, without the test.
> >
> > Testing hgweb's path handling is known to be extremely trick. We can't
> > reasonably emulate all the existing web environments and configs in our
> > test suite. So a doctest-style unit test is the right way to move
> > forward here. That means:
> >
> > a) making a purely functional helper function that does the path
> > manipulations (no environment or config inputs)
> > b) replace the current inline code with a call to the helper function
> > c) test all the interesting permutations in a doctest
> >
> > Here's what I've done on the default branch:
> >
> > diff -r 00862c7d3eec mercurial/hgweb/hgwebdir_mod.py
> > --- a/mercurial/hgweb/hgwebdir_mod.py   Mon Aug 01 12:54:00 2011 -0500
> > +++ b/mercurial/hgweb/hgwebdir_mod.py   Mon Aug 01 13:08:14 2011 -0500
> > @@ -51,6 +51,29 @@
> >         yield (prefix + '/' +
> >                util.pconvert(path[len(roothead):]).lstrip('/')).strip('/'),
> > path
> >
> > +def geturlcgivars(baseurl, port):
> > +    """
> > +    Extract CGI variables from baseurl
> > +
> > +    >>> geturlcgivars("http://host.org/base", "80")
> > +    ('host.org', '80', '/base')
> > +    >>> geturlcgivars("http://host.org:8000/base", "80")
> > +    ('host.org', '8000', '/base')
> > +    >>> geturlcgivars('/base', 8000)
> > +    ('', '8000', '/base')
> > +    >>> geturlcgivars("base", '8000')
> > +    ('', '8000', '/base')
> > +    """
> > +    u = util.url(baseurl)
> > +    name = u.host or ''
> > +    if u.port:
> > +        port = u.port
> > +    path = u.path
> > +    if not path.startswith('/'):
> > +        path = '/' + path
> > +
> > +    return name, str(port), path
> > +
> >  class hgwebdir(object):
> >     refreshinterval = 20
> >
> >
> Now that I look at the patch, I think the code should be:
>             path = u.path
>             if path is not None and not path.startswith('/'):
>                 path = '/' + path
>             elif path is None:
>                 path = "/"
> 
> This handles cases like 'http://host.org/' and 'http://host.org' correctly -
> especially the latter results in u.path being None, as in:
> 
> >>> from mercurial import util as u
> >>> url = u.url('http://host.org')
> >>> print url.path
> None
> 
> and the line 'if not path.startswith('/')' raises TypeError.
> What do you think?

Indeed, I'll fold a fix for that in.

-- 
Mathematics is the supreme nostalgia of our time.