From 51fba7d8f543e9a3d1c192f9a4e1fa9e29ccc998 Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 9 Apr 2014 15:57:50 +1000 Subject: [PATCH] html/template: fix two unrelated bugs 1) The code to catch an exception marked the template as escaped when it was not yet, which caused subsequent executions of the template to not escape properly. 2) ensurePipelineContains needs to handled Field as well as Identifier nodes. Fixes #7379. LGTM=mikesamuel R=mikesamuel CC=golang-codereviews https://golang.org/cl/85240043 --- src/pkg/html/template/escape.go | 51 ++++++++++++++++++++-------- src/pkg/html/template/escape_test.go | 32 +++++++++++++++++ 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/src/pkg/html/template/escape.go b/src/pkg/html/template/escape.go index 273bc1f190..5f458d172d 100644 --- a/src/pkg/html/template/escape.go +++ b/src/pkg/html/template/escape.go @@ -40,10 +40,14 @@ func escapeTemplates(tmpl *Template, names ...string) error { } return err } - tmpl.escaped = true - tmpl.Tree = tmpl.text.Tree } e.commit() + for _, name := range names { + if t := tmpl.set[name]; t != nil { + t.escaped = true + t.Tree = t.text.Tree + } + } return nil } @@ -207,6 +211,19 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context { return c } +// allIdents returns the names of the identifiers under the Ident field of the node, +// which might be a singleton (Identifier) or a slice (Field). +func allIdents(node parse.Node) []string { + switch node := node.(type) { + case *parse.IdentifierNode: + return []string{node.Ident} + case *parse.FieldNode: + return node.Ident + } + panic("unidentified node type in allIdents") + return nil +} + // ensurePipelineContains ensures that the pipeline has commands with // the identifiers in s in order. // If the pipeline already has some of the sanitizers, do not interfere. @@ -229,27 +246,31 @@ func ensurePipelineContains(p *parse.PipeNode, s []string) { idents = p.Cmds[i+1:] } dups := 0 - for _, id := range idents { - if escFnsEq(s[dups], (id.Args[0].(*parse.IdentifierNode)).Ident) { - dups++ - if dups == len(s) { - return + for _, idNode := range idents { + for _, ident := range allIdents(idNode.Args[0]) { + if escFnsEq(s[dups], ident) { + dups++ + if dups == len(s) { + return + } } } } newCmds := make([]*parse.CommandNode, n-len(idents), n+len(s)-dups) copy(newCmds, p.Cmds) // Merge existing identifier commands with the sanitizers needed. - for _, id := range idents { - pos := id.Args[0].Position() - i := indexOfStr((id.Args[0].(*parse.IdentifierNode)).Ident, s, escFnsEq) - if i != -1 { - for _, name := range s[:i] { - newCmds = appendCmd(newCmds, newIdentCmd(name, pos)) + for _, idNode := range idents { + pos := idNode.Args[0].Position() + for _, ident := range allIdents(idNode.Args[0]) { + i := indexOfStr(ident, s, escFnsEq) + if i != -1 { + for _, name := range s[:i] { + newCmds = appendCmd(newCmds, newIdentCmd(name, pos)) + } + s = s[i+1:] } - s = s[i+1:] } - newCmds = appendCmd(newCmds, id) + newCmds = appendCmd(newCmds, idNode) } // Create any remaining sanitizers. for _, name := range s { diff --git a/src/pkg/html/template/escape_test.go b/src/pkg/html/template/escape_test.go index 58383a6cd4..3ccf93ece0 100644 --- a/src/pkg/html/template/escape_test.go +++ b/src/pkg/html/template/escape_test.go @@ -1649,6 +1649,38 @@ func TestEmptyTemplate(t *testing.T) { } } +type Issue7379 int + +func (Issue7379) SomeMethod(x int) string { + return fmt.Sprintf("<%d>", x) +} + +// This is a test for issue 7379: type assertion error caused panic, and then +// the code to handle the panic breaks escaping. It's hard to see the second +// problem once the first is fixed, but its fix is trivial so we let that go. See +// the discussion for issue 7379. +func TestPipeToMethodIsEscaped(t *testing.T) { + tmpl := Must(New("x").Parse("{{0 | .SomeMethod}}\n")) + tryExec := func() string { + defer func() { + panicValue := recover() + if panicValue != nil { + t.Errorf("panicked: %v\n", panicValue) + } + }() + var b bytes.Buffer + tmpl.Execute(&b, Issue7379(0)) + return b.String() + } + for i := 0; i < 3; i++ { + str := tryExec() + const expect = "<0>\n" + if str != expect { + t.Errorf("expected %q got %q", expect, str) + } + } +} + func BenchmarkEscapedExecute(b *testing.B) { tmpl := Must(New("t").Parse(`{{.}}`)) var buf bytes.Buffer -- 2.48.1