html/template: only search identifier nodes for predefined escapers

Predefined escapers (i.e. "html" and "urlquery") should only occur in
Identifier nodes, and never in Field or Chain nodes, since these are
global functions that return string values (see inline comments for more
details). Therefore, skip Chain and Field nodes when searching for
predefined escapers in template pipelines.

Also, make a non-functional change two existing test cases to avoid
giving the impression that it is valid to reference a field of a
predefined escaper.

Fixes #20323

Change-Id: I34f722f443c778699fcdd575dc3e0fd1fd6f2eb3
Reviewed-on: https://go-review.googlesource.com/43296
Reviewed-by: Samuel Tan <samueltan@google.com>
Reviewed-by: Mike Samuel <mikesamuel@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index 3037e07b296c5ba2a2aef46a20a989766172385e..92b1d086778e5da614fa0168a5ddf836e249f950 100644 (file)
--- a/src/html/template/escape.go
+++ b/src/html/template/escape.go
@@ -139,20 +139,6 @@ func (e *escaper) escape(c context, n parse.Node) context {
        panic("escaping " + n.String() + " is unimplemented")
 }
 
-// allIdents returns the names of the identifiers under the Ident field of the node,
-// 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
-       }
-       return nil
-}
-
 // escapeAction escapes an action template node.
 func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
        if len(n.Pipe.Decl) != 0 {
@@ -162,14 +148,24 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
        c = nudge(c)
        // Check for disallowed use of predefined escapers in the pipeline.
        for pos, idNode := range n.Pipe.Cmds {
-               for _, ident := range allIdents(idNode.Args[0]) {
-                       if _, ok := predefinedEscapers[ident]; ok {
-                               if pos < len(n.Pipe.Cmds)-1 ||
-                                       c.state == stateAttr && c.delim == delimSpaceOrTagEnd && ident == "html" {
-                                       return context{
-                                               state: stateError,
-                                               err:   errorf(ErrPredefinedEscaper, n, n.Line, "predefined escaper %q disallowed in template", ident),
-                                       }
+               node, ok := idNode.Args[0].(*parse.IdentifierNode)
+               if !ok {
+                       // A predefined escaper "esc" will never be found as an identifier in a
+                       // Chain or Field node, since:
+                       // - "esc.x ..." is invalid, since predefined escapers return strings, and
+                       //   strings do not have methods, keys or fields.
+                       // - "... .esc" is invalid, since predefined escapers are global functions,
+                       //   not methods or fields of any types.
+                       // Therefore, it is safe to ignore these two node types.
+                       continue
+               }
+               ident := node.Ident
+               if _, ok := predefinedEscapers[ident]; ok {
+                       if pos < len(n.Pipe.Cmds)-1 ||
+                               c.state == stateAttr && c.delim == delimSpaceOrTagEnd && ident == "html" {
+                               return context{
+                                       state: stateError,
+                                       err:   errorf(ErrPredefinedEscaper, n, n.Line, "predefined escaper %q disallowed in template", ident),
                                }
                        }
                }

diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index 865226f855108b1159be59212a689b93bb966f93..d61683b8c99c5eb6b4bdf8008f85ffbbc794ced7 100644 (file)
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -685,6 +685,40 @@ func TestEscape(t *testing.T) {
        }
 }
 
+func TestEscapeMap(t *testing.T) {
+       data := map[string]string{
+               "html":     `<h1>Hi!</h1>`,
+               "urlquery": `http://www.foo.com/index.html?title=main`,
+       }
+       for _, test := range [...]struct {
+               desc, input, output string
+       }{
+               // covering issue 20323
+               {
+                       "field with predefined escaper name 1",
+                       `{{.html | print}}`,
+                       `&lt;h1&gt;Hi!&lt;/h1&gt;`,
+               },
+               // covering issue 20323
+               {
+                       "field with predefined escaper name 2",
+                       `{{.urlquery | print}}`,
+                       `http://www.foo.com/index.html?title=main`,
+               },
+       } {
+               tmpl := Must(New("").Parse(test.input))
+               b := new(bytes.Buffer)
+               if err := tmpl.Execute(b, data); err != nil {
+                       t.Errorf("%s: template execution failed: %s", test.desc, err)
+                       continue
+               }
+               if w, g := test.output, b.String(); w != g {
+                       t.Errorf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.desc, w, g)
+                       continue
+               }
+       }
+}
+
 func TestEscapeSet(t *testing.T) {
        type dataItem struct {
                Children []*dataItem
@@ -1595,14 +1629,14 @@ func TestEnsurePipelineContains(t *testing.T) {
                },
                {
                        // covering issue 10801
-                       "{{.X | js.x }}",
-                       ".X | js.x | urlquery | html",
+                       "{{.X | println.x }}",
+                       ".X | println.x | urlquery | html",
                        []string{"urlquery", "html"},
                },
                {
                        // covering issue 10801
-                       "{{.X | (print 12 | js).x }}",
-                       ".X | (print 12 | js).x | urlquery | html",
+                       "{{.X | (print 12 | println).x }}",
+                       ".X | (print 12 | println).x | urlquery | html",
                        []string{"urlquery", "html"},
                },
                // The following test cases ensure that the merging of internal escapers