[PATCH 3 of 3 V4] hgweb: add revsetsearch() function when query can be parsed as a revset

[Kevin Bullock]

On 4 Sep 2013, at 2:12 PM, Alexander Plavin wrote:

> # HG changeset patch
> # User Alexander Plavin <alexander at plav.in>
> # Date 1375823774 -14400
> #      Wed Aug 07 01:16:14 2013 +0400
> # Node ID b9c27aecca8521161a28279e595e7d6aca82245d
> # Parent  327bee4dc0ccf28853b979072187989c4abf82df
> hgweb: add revsetsearch() function when query can be parsed as a revset
> 
> This function is used when all the conditions are met:
> - 'reverse(%s)' % query string can be parsed to a revset tree
> - this tree has depth more than two, i.e. the query has some part of
> revset syntax used
> - the repo can be actually matched against this tree, i.e. it has only existent
> function/operators and revisions/tags/bookmarks specified are correct
> 
> Otherwise keywordsearch() or revsearch() functions are used as before.
> 
> Add several new tests for different parsing conditions and exception handling.
> 
> diff -r 327bee4dc0cc -r b9c27aecca85 mercurial/hgweb/webcommands.py
> --- a/mercurial/hgweb/webcommands.py	Fri Aug 09 22:52:58 2013 +0400
> +++ b/mercurial/hgweb/webcommands.py	Wed Aug 07 01:16:14 2013 +0400
> @@ -16,6 +16,8 @@
> from mercurial import help as helpmod
> from mercurial import scmutil
> from mercurial.i18n import _
> +from mercurial.error import ParseError, RepoLookupError, Abort
> +from mercurial import parser, revset
> 
> # __all__ is populated with the allowed commands. Be sure to add to it if
> # you're adding a new command, or the new command won't work.
> @@ -111,6 +113,7 @@
> def _search(web, req, tmpl):
>     MODE_REVISION = 'rev'
>     MODE_KEYWORD = 'keyword'
> +    MODE_REVSET = 'revset'
> 
>     def revsearch(ctx):
>         yield ctx
> @@ -143,19 +146,50 @@
> 
>             yield ctx
> 
> +    def revsetsearch(revdef):
> +        revs = revset.match(web.repo.ui, revdef)(web.repo, list(web.repo))
> +        for r in revs:
> +            yield web.repo[r]
> +
>     searchfuncs = {
>         MODE_REVISION: revsearch,
>         MODE_KEYWORD: keywordsearch,
> +        MODE_REVSET: revsetsearch,
>     }
> 
>     def getsearchmode(query):
>         try:
>             ctx = web.repo[query]
>         except (error.RepoError, error.LookupError):
> -            return MODE_KEYWORD, query
> +            # query is not an exact revision pointer, need to
> +            # decide if it's a revset expession or keywords
> +            pass
>         else:
>             return MODE_REVISION, ctx
> 
> +        revdef = 'reverse(%s)' % query
> +        try:
> +            p = parser.parser(revset.tokenize, revset.elements)
> +            tree, pos = p.parse(revdef)
> +        except ParseError:
> +            # can't parse to a revset tree
> +            return MODE_KEYWORD, query

revset.parse() would be sufficient here.

Moreover, I'm a little uneasy queuing this before we have a whitelist of allowed expressions. That is, I'd rather see a series that, in this order:

1. Adds the appropriate helpers to revset.py (with more complete commit descriptions saying how they'll be used)
2. Adds the whitelist with more detail about the criteria for 'heavyweight' functions (e.g. functions that just return a lot of changesets don't count).
3. Adds revset search in a single patch, with regexes and non-whitelisted functions disabled from the start.
4. Adds the config knob, along with a big fat warning in the documentation about the risks of setting it to False.

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock