| 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 <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.
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):