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

diff --git a/src/pkg/html/template/escape.go b/src/pkg/html/template/escape.go
index 273bc1f190bef2ae92ee8130042bbda8df4803dd..5f458d172d7ee488f2e22d6c0ec98c90c311ab6c 100644 (file)
--- 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 58383a6cd4e291454d659776b8fca56d8778face..3ccf93ece01877a2563f47702ad83f7bb919eefc 100644 (file)
--- 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>&lt;0&gt;</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