html/template: prevent panic when escaping actions involving chain nodes

The current escape code panics when an action involves chain nodes.
Such nodes can be seen in the following situation:

{{ . | AAA.B }} - AAA being a registered function

The above expression is actually valid, because AAA could return a
map containing a B key. The tests in text/template explicitly
demonstrate this case.

Fix allIdents to cover also chain nodes.

While I was investigating this issue, I realized that the tests
introduced in similar CL 9621 were incorrect. Parse errors were
caught as expected, but for the wrong reason. Fixed them as well.
No changes in text/template code itself.

Fixes #10801

Change-Id: Ic9fe43b63669298ca52c3f499e2725dd2bb818a8
Reviewed-on: https://go-review.googlesource.com/10340
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index a9529446dd0d753292900a9646cf7164dc9e2768..bfcea66b905c63d972c231a454eb39253567084e 100644 (file)
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -205,13 +205,15 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
 }
 
 // allIdents returns the names of the identifiers under the Ident field of the node,
-// which might be a singleton (Identifier) or a slice (Field).
+// which might be a singleton (Identifier) or a slice (Field or Chain).
 func allIdents(node parse.Node) []string {
        switch node := node.(type) {
        case *parse.IdentifierNode:
                return []string{node.Ident}
        case *parse.FieldNode:
                return node.Ident
+       case *parse.ChainNode:
+               return node.Field
        }
        panic("unidentified node type in allIdents")
 }

diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index 6729ebf4a780f42ba11e664c1c71e0361d03b1b0..41ab0c8ae7bac62166ac4a6a9cfb3fcf0eaf6d28 100644 (file)
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -1557,6 +1557,18 @@ func TestEnsurePipelineContains(t *testing.T) {
                        ".X | urlquery | html | print 2 | .f 3",
                        []string{"urlquery", "html"},
                },
+               {
+                       // covering issue 10801
+                       "{{.X | js.x }}",
+                       ".X | js.x | urlquery | html",
+                       []string{"urlquery", "html"},
+               },
+               {
+                       // covering issue 10801
+                       "{{.X | (print 12 | js).x }}",
+                       ".X | (print 12 | js).x | urlquery | html",
+                       []string{"urlquery", "html"},
+               },
        }
        for i, test := range tests {
                tmpl := template.Must(template.New("test").Parse(test.input))

diff --git a/src/text/template/parse/parse_test.go b/src/text/template/parse/parse_test.go
index 9e62bd2df6a1aa14191bfa1be2c126d25da6e6c1..200d50c194dbd0cf2da3a8fd2fe18f227566e58d 100644 (file)
--- a/src/text/template/parse/parse_test.go
+++ b/src/text/template/parse/parse_test.go
@@ -272,8 +272,8 @@ var parseTests = []parseTest{
        // Wrong pipeline
        {"wrong pipeline dot", "{{12|.}}", hasError, ""},
        {"wrong pipeline number", "{{.|12|printf}}", hasError, ""},
-       {"wrong pipeline string", "{{.|print|\"error\"}}", hasError, ""},
-       {"wrong pipeline char", "{{12|print|html|'e'}}", hasError, ""},
+       {"wrong pipeline string", "{{.|printf|\"error\"}}", hasError, ""},
+       {"wrong pipeline char", "{{12|printf|'e'}}", hasError, ""},
        {"wrong pipeline boolean", "{{.|true}}", hasError, ""},
        {"wrong pipeline nil", "{{'c'|nil}}", hasError, ""},
        {"empty pipeline", `{{printf "%d" ( ) }}`, hasError, ""},