Patchwork [STABLE] templater: catch parsing error for sub

login
register
mail settings
Submitter Sean Farley
Date May 13, 2013, 5:46 p.m.
Message ID <1e85222b91951651ea8f.1368467192@laptop.local>
Download mbox | patch
Permalink /patch/1618/
State Changes Requested, archived
Headers show

Comments

Sean Farley - May 13, 2013, 5:46 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1368464142 18000
#      Mon May 13 11:55:42 2013 -0500
# Branch stable
# Node ID 1e85222b91951651ea8f8fc3de1af4320a79bd7c
# Parent  12dbdd348bb0977366200bf96cb6d2afa85faf13
templater: catch parsing error for sub

This follows d8d548d868d3, which added template expansion for the sub function,
to fix the case where the description contained a curly brace (i.e. not part of
any template). This patch simply wraps the templater call into a try statement.

Test coverage has been added.
Matt Mackall - May 13, 2013, 10:34 p.m.
On Mon, 2013-05-13 at 12:46 -0500, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1368464142 18000
> #      Mon May 13 11:55:42 2013 -0500
> # Branch stable
> # Node ID 1e85222b91951651ea8f8fc3de1af4320a79bd7c
> # Parent  12dbdd348bb0977366200bf96cb6d2afa85faf13
> templater: catch parsing error for sub
> 
> This follows d8d548d868d3, which added template expansion for the sub function,
> to fix the case where the description contained a curly brace (i.e. not part of
> any template). This patch simply wraps the templater call into a try statement.
> 
> Test coverage has been added.

...

> -  $ hg tag -r 3 -m at3 -d '10 0' at3
> +  $ hg tag -r 3 -m 'at3 with {' -d '10 0' at3

> +Test the sub function of templating that doesn't throw an error:
> +
> +  $ hg log -R latesttag -r 10 --template '{sub("{", r"\{", desc)}\n'
> +  at3 with \{

Hmm, something is deeply wrong here. We should not be interpreting
commit messages as templates, ever. That's how you get XSS attacks.

In the earler patch you have:

+  $ hg log -R latesttag -r 10 --template '{sub("[0-9]", "x",
"{rev}")}\n'

That's great, that's a literal template. Expand away. But here we've
got:

+  $ hg log -R latesttag -r 10 --template '{sub("{", r"\{", desc)}\n'

..which suggests we're dereferencing the value of desc and expanding it
as a template, which is bad bad bad. Perhaps we need a helper function
that:

- looks at the arg
- if symbol, return its value
- if literal string, run the template and return as a string

Patch

diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -249,12 +249,15 @@ 
         raise error.ParseError(_("sub expects three arguments"))
 
     pat = stringify(args[0][0](context, mapping, args[0][1]))
     rpl = stringify(args[1][0](context, mapping, args[1][1]))
     src = stringify(args[2][0](context, mapping, args[2][1]))
-    src = stringify(runtemplate(context, mapping,
-                                compiletemplate(src, context)))
+    try:
+        src = stringify(runtemplate(context, mapping,
+                                    compiletemplate(src, context)))
+    except error.ParseError:
+        pass
     yield re.sub(pat, rpl, src)
 
 def if_(context, mapping, args):
     if not (2 <= len(args) <= 3):
         # i18n: "if" is a keyword
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -1474,11 +1474,11 @@ 
   0: null+1
 
 Merged tag overrides:
 
   $ hg tag -r 5 -m t5 -d '9 0' t5
-  $ hg tag -r 3 -m at3 -d '10 0' at3
+  $ hg tag -r 3 -m 'at3 with {' -d '10 0' at3
   $ hg log --template '{rev}: {latesttag}+{latesttagdistance}\n'
   10: t5+5
   9: t5+4
   8: t5+3
   7: t5+2
@@ -1508,11 +1508,11 @@ 
   > [ui]
   > style = ~/styles/teststyle
   > EOF
 
   $ hg -R latesttag tip
-  test 10:dee8f28249af
+  test 10:f55bb88e0f02
 
 Test recursive showlist template (issue1989):
 
   $ cat > style1989 <<EOF
   > changeset = '{file_mods}{manifest}{extras}'
@@ -1533,5 +1533,10 @@ 
   
 Test the sub function of templating for expansion:
 
   $ hg log -R latesttag -r 10 --template '{sub("[0-9]", "x", "{rev}")}\n'
   xx
+
+Test the sub function of templating that doesn't throw an error:
+
+  $ hg log -R latesttag -r 10 --template '{sub("{", r"\{", desc)}\n'
+  at3 with \{