diff options
| author | Rob Pike <r@golang.org> | 2014-04-09 15:57:50 +1000 |
|---|---|---|
| committer | Rob Pike <r@golang.org> | 2014-04-09 15:57:50 +1000 |
| commit | f71b8ac830902415ca2a0971b832ca75d2209a1f (patch) | |
| tree | e78a28f604349f56528142464a291c14a158c225 | |
| parent | b2a0e3088ef0056df2f5c1c52f11dff932f29995 (diff) | |
| download | go-f71b8ac830902415ca2a0971b832ca75d2209a1f.tar.gz | |
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 issue 7379.
LGTM=mikesamuel
R=mikesamuel
CC=golang-codereviews
https://codereview.appspot.com/85240043
| -rw-r--r-- | src/pkg/html/template/escape.go | 51 | ||||
| -rw-r--r-- | 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 273bc1f19..5f458d172 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 58383a6cd..3ccf93ece 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("<html>{{0 | .SomeMethod}}</html>\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 = "<html><0></html>\n" + if str != expect { + t.Errorf("expected %q got %q", expect, str) + } + } +} + func BenchmarkEscapedExecute(b *testing.B) { tmpl := Must(New("t").Parse(`<a onclick="alert('{{.}}')">{{.}}</a>`)) var buf bytes.Buffer |