Skip to content

Commit f685336

Browse files
dspeziarobpike
authored andcommitted
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 <[email protected]>
1 parent ae38ef4 commit f685336

File tree

3 files changed

+17
-3
lines changed

3 files changed

+17
-3
lines changed

src/html/template/escape.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,15 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
205205
}
206206

207207
// allIdents returns the names of the identifiers under the Ident field of the node,
208-
// which might be a singleton (Identifier) or a slice (Field).
208+
// which might be a singleton (Identifier) or a slice (Field or Chain).
209209
func allIdents(node parse.Node) []string {
210210
switch node := node.(type) {
211211
case *parse.IdentifierNode:
212212
return []string{node.Ident}
213213
case *parse.FieldNode:
214214
return node.Ident
215+
case *parse.ChainNode:
216+
return node.Field
215217
}
216218
panic("unidentified node type in allIdents")
217219
}

src/html/template/escape_test.go

+12
Original file line numberDiff line numberDiff line change
@@ -1557,6 +1557,18 @@ func TestEnsurePipelineContains(t *testing.T) {
15571557
".X | urlquery | html | print 2 | .f 3",
15581558
[]string{"urlquery", "html"},
15591559
},
1560+
{
1561+
// covering issue 10801
1562+
"{{.X | js.x }}",
1563+
".X | js.x | urlquery | html",
1564+
[]string{"urlquery", "html"},
1565+
},
1566+
{
1567+
// covering issue 10801
1568+
"{{.X | (print 12 | js).x }}",
1569+
".X | (print 12 | js).x | urlquery | html",
1570+
[]string{"urlquery", "html"},
1571+
},
15601572
}
15611573
for i, test := range tests {
15621574
tmpl := template.Must(template.New("test").Parse(test.input))

src/text/template/parse/parse_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ var parseTests = []parseTest{
272272
// Wrong pipeline
273273
{"wrong pipeline dot", "{{12|.}}", hasError, ""},
274274
{"wrong pipeline number", "{{.|12|printf}}", hasError, ""},
275-
{"wrong pipeline string", "{{.|print|\"error\"}}", hasError, ""},
276-
{"wrong pipeline char", "{{12|print|html|'e'}}", hasError, ""},
275+
{"wrong pipeline string", "{{.|printf|\"error\"}}", hasError, ""},
276+
{"wrong pipeline char", "{{12|printf|'e'}}", hasError, ""},
277277
{"wrong pipeline boolean", "{{.|true}}", hasError, ""},
278278
{"wrong pipeline nil", "{{'c'|nil}}", hasError, ""},
279279
{"empty pipeline", `{{printf "%d" ( ) }}`, hasError, ""},

0 commit comments

Comments
 (0)