Patchwork hgweb: add support for colon-separated revision format in search

login
register
mail settings
Submitter Alexander Plavin
Date July 15, 2013, 10:53 p.m.
Message ID <52803877038ae1a817e4.1373928810@debian-alexander.dolgopa>
Download mbox | patch
Permalink /patch/1904/
State Superseded
Headers show

Comments

Alexander Plavin - July 15, 2013, 10:53 p.m.
# HG changeset patch
# User Alexander Plavin <me@aplavin.ru>
# Date 1373830561 -14400
#      Sun Jul 14 23:36:01 2013 +0400
# Node ID 52803877038ae1a817e467fb8d49cb584b984742
# Parent  390ff286651b30b2a5f71a1901bb34aefec1c6f9
hgweb: add support for colon-separated revision format in search

Now it's possible to specify a revision in the format
<definion 1>:<definition 2>, if <definion 1> and <definition 2> point to
the same place, like in other parts of hg.
Martin Geisler - July 16, 2013, 12:01 p.m.
Alexander Plavin <me@aplavin.ru> writes:

> # HG changeset patch
> # User Alexander Plavin <me@aplavin.ru>
> # Date 1373830561 -14400
> #      Sun Jul 14 23:36:01 2013 +0400
> # Node ID 52803877038ae1a817e467fb8d49cb584b984742
> # Parent  390ff286651b30b2a5f71a1901bb34aefec1c6f9
> hgweb: add support for colon-separated revision format in search
>
> Now it's possible to specify a revision in the format
> <definion 1>:<definition 2>, if <definion 1> and <definition 2> point to
> the same place, like in other parts of hg.

In other parts of Mercurial, this works because "x:y" means "changesets
with revision numbers between x and y" (see "hg help revsets").

The parsing of "x:y" is done by the revset parser, see revset.rangeset.
I would have imagined that the search in hgweb would use the same parser
so you avoid hard-coding parts of the revset syntax (specifically ":")
in hgweb.

Have you tried asking the Bitbucket guys (such as Brodie) if we can get
their backend code that restricts revsets to a safe subset? They might
be interested in open sourcing that code.

Alternatively, I think it should be possible to implement it again by
recursively searching through the parsed revset query (the parse tree)
and only allow queries where all functions are on a whitelist.
Alexander Plavin - July 17, 2013, 9:50 a.m.
2013/7/16 Martin Geisler <martin@geisler.net>:
> Alexander Plavin <me@aplavin.ru> writes:
>
>> # HG changeset patch
>> # User Alexander Plavin <me@aplavin.ru>
>> # Date 1373830561 -14400
>> #      Sun Jul 14 23:36:01 2013 +0400
>> # Node ID 52803877038ae1a817e467fb8d49cb584b984742
>> # Parent  390ff286651b30b2a5f71a1901bb34aefec1c6f9
>> hgweb: add support for colon-separated revision format in search
>>
>> Now it's possible to specify a revision in the format
>> <definion 1>:<definition 2>, if <definion 1> and <definition 2> point to
>> the same place, like in other parts of hg.
>
> In other parts of Mercurial, this works because "x:y" means "changesets
> with revision numbers between x and y" (see "hg help revsets").
>
> The parsing of "x:y" is done by the revset parser, see revset.rangeset.
> I would have imagined that the search in hgweb would use the same parser
> so you avoid hard-coding parts of the revset syntax (specifically ":")
> in hgweb.

Yes, I know this, but there is a difference in hgweb search. When you
type a direct pointer to a specific revision (number or hash), then
you have the log starting with this revision (and not only one
revision, which you've typed). In other cases, a search is performed,
and only matched revisions are shown (not all the log starting from
them). So, we can't simply use that piece of code (I've explored
revrange function and related).

>
> Have you tried asking the Bitbucket guys (such as Brodie) if we can get
> their backend code that restricts revsets to a safe subset? They might
> be interested in open sourcing that code.
>
> Alternatively, I think it should be possible to implement it again by
> recursively searching through the parsed revset query (the parse tree)
> and only allow queries where all functions are on a whitelist.

I'm planning to implement it this way (as there were no objections to
my questions about it on the ML), and it's almost clear how to do
this.

>
> --
> Martin Geisler

Patch

diff -r 390ff286651b -r 52803877038a mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py	Sun Jul 14 05:35:04 2013 +0400
+++ b/mercurial/hgweb/webcommands.py	Sun Jul 14 23:36:01 2013 +0400
@@ -194,9 +194,26 @@ 
             hi = query
         else:
             hi = 'tip'
+
+        dosearch = False
         try:
-            ctx = web.repo[hi]
-        except (error.RepoError, error.LookupError):
+            hiparts = hi.split(':')
+            if len(hiparts) == 1:
+                ctx = web.repo[hi]
+            elif len(hiparts) == 2:
+                # two parts - check they unambiguously point to the same rev
+                ctx1 = web.repo[hiparts[0]]
+                ctx2 = web.repo[hiparts[1]]
+                if ctx1 == ctx2:
+                    ctx = ctx1
+                else:
+                    dosearch = True
+            else:
+                dosearch = True
+        except (error.RepoError, error.LookupError) as e:
+            dosearch = True
+
+        if dosearch:
             return _search(web, req, tmpl) # XXX redirect to 404 page?
 
     def changelist(latestonly, **map):