diff --git a/src/html/template/escape.go b/src/html/template/escape.go index 92b1d08677..b51a37039b 100644 --- a/src/html/template/escape.go +++ b/src/html/template/escape.go @@ -19,8 +19,7 @@ import ( // been modified. Otherwise the named templates have been rendered // unusable. func escapeTemplate(tmpl *Template, node parse.Node, name string) error { - e := newEscaper(tmpl) - c, _ := e.escapeTree(context{}, node, name, 0) + c, _ := tmpl.esc.escapeTree(context{}, node, name, 0) var err error if c.err != nil { err, c.err.Name = c.err, name @@ -36,7 +35,7 @@ func escapeTemplate(tmpl *Template, node parse.Node, name string) error { } return err } - e.commit() + tmpl.esc.commit() if t := tmpl.set[name]; t != nil { t.escapeErr = escapeOK t.Tree = t.text.Tree @@ -81,7 +80,8 @@ var funcMap = template.FuncMap{ // escaper collects type inferences about templates and changes needed to make // templates injection safe. type escaper struct { - tmpl *Template + // ns is the nameSpace that this escaper is associated with. + ns *nameSpace // output[templateName] is the output context for a templateName that // has been mangled to include its input context. output map[string]context @@ -98,10 +98,10 @@ type escaper struct { textNodeEdits map[*parse.TextNode][]byte } -// newEscaper creates a blank escaper for the given set. -func newEscaper(t *Template) *escaper { - return &escaper{ - t, +// makeEscaper creates a blank escaper for the given set. +func makeEscaper(n *nameSpace) escaper { + return escaper{ + n, map[string]context{}, map[string]*template.Template{}, map[string]bool{}, @@ -491,13 +491,13 @@ func (e *escaper) escapeList(c context, n *parse.ListNode) context { // It returns the best guess at an output context, and the result of the filter // which is the same as whether e was updated. func (e *escaper) escapeListConditionally(c context, n *parse.ListNode, filter func(*escaper, context) bool) (context, bool) { - e1 := newEscaper(e.tmpl) + e1 := makeEscaper(e.ns) // Make type inferences available to f. for k, v := range e.output { e1.output[k] = v } c = e1.escapeList(c, n) - ok := filter != nil && filter(e1, c) + ok := filter != nil && filter(&e1, c) if ok { // Copy inferences and edits from e1 back into e. for k, v := range e1.output { @@ -546,7 +546,7 @@ func (e *escaper) escapeTree(c context, node parse.Node, name string, line int) if t == nil { // Two cases: The template exists but is empty, or has never been mentioned at // all. Distinguish the cases in the error messages. - if e.tmpl.set[name] != nil { + if e.ns.set[name] != nil { return context{ state: stateError, err: errorf(ErrNoSuchTemplate, node, line, "%q is an incomplete or empty template", name), @@ -794,8 +794,11 @@ func (e *escaper) commit() { for name := range e.output { e.template(name).Funcs(funcMap) } + // Any template from the name space associated with this escaper can be used + // to add derived templates to the underlying text/template name space. + tmpl := e.arbitraryTemplate() for _, t := range e.derived { - if _, err := e.tmpl.text.AddParseTree(t.Name(), t.Tree); err != nil { + if _, err := tmpl.text.AddParseTree(t.Name(), t.Tree); err != nil { panic("error adding derived template") } } @@ -808,17 +811,34 @@ func (e *escaper) commit() { for n, s := range e.textNodeEdits { n.Text = s } + // Reset state that is specific to this commit so that the same changes are + // not re-applied to the template on subsequent calls to commit. + e.called = make(map[string]bool) + e.actionNodeEdits = make(map[*parse.ActionNode][]string) + e.templateNodeEdits = make(map[*parse.TemplateNode]string) + e.textNodeEdits = make(map[*parse.TextNode][]byte) } // template returns the named template given a mangled template name. func (e *escaper) template(name string) *template.Template { - t := e.tmpl.text.Lookup(name) + // Any template from the name space associated with this escaper can be used + // to look up templates in the underlying text/template name space. + t := e.arbitraryTemplate().text.Lookup(name) if t == nil { t = e.derived[name] } return t } +// arbitraryTemplate returns an arbitrary template from the name space +// associated with e and panics if no templates are found. +func (e *escaper) arbitraryTemplate() *Template { + for _, t := range e.ns.set { + return t + } + panic("no templates in name space") +} + // Forwarding functions so that clients need only import this package // to reach the general escaping functions of text/template. diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go index d61683b8c9..f5a4ce1736 100644 --- a/src/html/template/escape_test.go +++ b/src/html/template/escape_test.go @@ -1564,7 +1564,7 @@ func TestEscapeText(t *testing.T) { } for _, test := range tests { - b, e := []byte(test.input), newEscaper(nil) + b, e := []byte(test.input), makeEscaper(nil) c := e.escapeText(context{}, &parse.TextNode{NodeType: parse.NodeText, Text: b}) if !test.output.eq(c) { t.Errorf("input %q: want context\n\t%v\ngot\n\t%v", test.input, test.output, c) @@ -1671,29 +1671,21 @@ func TestEnsurePipelineContains(t *testing.T) { ".X | html", []string{"_html_template_rcdataescaper"}, }, - { - "{{.X | html}}", - ".X | html | html", - []string{"_html_template_htmlescaper", "_html_template_attrescaper"}, - }, - { - "{{.X | html}}", - ".X | html | html", - []string{"_html_template_rcdataescaper", "_html_template_attrescaper"}, - }, } for i, test := range tests { tmpl := template.Must(template.New("test").Parse(test.input)) action, ok := (tmpl.Tree.Root.Nodes[0].(*parse.ActionNode)) if !ok { - t.Errorf("#%d: First node is not an action: %s", i, test.input) + t.Errorf("First node is not an action: %s", test.input) continue } pipe := action.Pipe + originalIDs := make([]string, len(test.ids)) + copy(originalIDs, test.ids) ensurePipelineContains(pipe, test.ids) got := pipe.String() if got != test.output { - t.Errorf("#%d: %s, %v: want\n\t%s\ngot\n\t%s", i, test.input, test.ids, test.output, got) + t.Errorf("#%d: %s, %v: want\n\t%s\ngot\n\t%s", i, test.input, originalIDs, test.output, got) } } } @@ -1855,6 +1847,40 @@ func TestErrorOnUndefined(t *testing.T) { } } +// This covers issue #20842. +func TestIdempotentExecute(t *testing.T) { + tmpl := Must(New(""). + Parse(`{{define "main"}}
{{template "hello"}}{{end}}`)) + Must(tmpl. + Parse(`{{define "hello"}}Hello, {{"Ladies & Gentlemen!"}}{{end}}`)) + got := new(bytes.Buffer) + var err error + // Ensure that "hello" produces the same output when executed twice. + want := "Hello, Ladies & Gentlemen!" + for i := 0; i < 2; i++ { + err = tmpl.ExecuteTemplate(got, "hello", nil) + if err != nil { + t.Errorf("unexpected error: %s", err) + } + if got.String() != want { + t.Errorf("after executing template \"hello\", got:\n\t%q\nwant:\n\t%q\n", got.String(), want) + } + got.Reset() + } + // Ensure that the implicit re-execution of "hello" during the execution of + // "main" does not cause the output of "hello" to change. + err = tmpl.ExecuteTemplate(got, "main", nil) + if err != nil { + t.Errorf("unexpected error: %s", err) + } + // If the HTML escaper is added again to the action {{"Ladies & Gentlemen!"}}, + // we would expected to see the ampersand overescaped to "&". + want = "Hello, Ladies & Gentlemen!" + if got.String() != want { + t.Errorf("after executing template \"main\", got:\n\t%q\nwant:\n\t%q\n", got.String(), want) + } +} + func BenchmarkEscapedExecute(b *testing.B) { tmpl := Must(New("t").Parse(`{{.}}`)) var buf bytes.Buffer diff --git a/src/html/template/template.go b/src/html/template/template.go index 246ef04dbe..6a661bf6e5 100644 --- a/src/html/template/template.go +++ b/src/html/template/template.go @@ -36,6 +36,7 @@ type nameSpace struct { mu sync.Mutex set map[string]*Template escaped bool + esc escaper } // Templates returns a slice of the templates associated with t, including t @@ -250,13 +251,13 @@ func (t *Template) Clone() (*Template, error) { if err != nil { return nil, err } + ns := &nameSpace{set: make(map[string]*Template)} + ns.esc = makeEscaper(ns) ret := &Template{ nil, textClone, textClone.Tree, - &nameSpace{ - set: make(map[string]*Template), - }, + ns, } ret.set[ret.Name()] = ret for _, x := range textClone.Templates() { @@ -279,13 +280,13 @@ func (t *Template) Clone() (*Template, error) { // New allocates a new HTML template with the given name. func New(name string) *Template { + ns := &nameSpace{set: make(map[string]*Template)} + ns.esc = makeEscaper(ns) tmpl := &Template{ nil, template.New(name), nil, - &nameSpace{ - set: make(map[string]*Template), - }, + ns, } tmpl.set[name] = tmpl return tmpl