From 0fee63351d0a41fc979e4480460e5aa76903bab6 Mon Sep 17 00:00:00 2001 From: Andrew Gerrand Date: Tue, 19 Aug 2014 14:24:14 +1000 Subject: [PATCH] html/template: don't panic on second execution of unescapable template Fixes #8431. LGTM=r R=golang-codereviews, r, minux CC=golang-codereviews https://golang.org/cl/130830043 --- src/pkg/html/template/escape.go | 3 ++- src/pkg/html/template/escape_test.go | 5 ++++ src/pkg/html/template/template.go | 36 +++++++++++++++++----------- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/pkg/html/template/escape.go b/src/pkg/html/template/escape.go index 4e379828d4..3ba3747f6f 100644 --- a/src/pkg/html/template/escape.go +++ b/src/pkg/html/template/escape.go @@ -34,6 +34,7 @@ func escapeTemplates(tmpl *Template, names ...string) error { // Prevent execution of unsafe templates. for _, name := range names { if t := tmpl.set[name]; t != nil { + t.escapeErr = err t.text.Tree = nil t.Tree = nil } @@ -44,7 +45,7 @@ func escapeTemplates(tmpl *Template, names ...string) error { e.commit() for _, name := range names { if t := tmpl.set[name]; t != nil { - t.escaped = true + t.escapeErr = escapeOK t.Tree = t.text.Tree } } diff --git a/src/pkg/html/template/escape_test.go b/src/pkg/html/template/escape_test.go index 3ccf93ece0..5509b836f9 100644 --- a/src/pkg/html/template/escape_test.go +++ b/src/pkg/html/template/escape_test.go @@ -994,6 +994,11 @@ func TestErrors(t *testing.T) { t.Errorf("input=%q: error\n\t%q\ndoes not contain expected string\n\t%q", test.input, got, test.err) continue } + // Check that we get the same error if we call Execute again. + if err := tmpl.Execute(buf, nil); err == nil || err.Error() != got { + t.Errorf("input=%q: unexpected error on second call %q", test.input, err) + + } } } diff --git a/src/pkg/html/template/template.go b/src/pkg/html/template/template.go index d389658979..538837cc5c 100644 --- a/src/pkg/html/template/template.go +++ b/src/pkg/html/template/template.go @@ -17,7 +17,8 @@ import ( // Template is a specialized Template from "text/template" that produces a safe // HTML document fragment. type Template struct { - escaped bool + // Sticky error if escaping fails. + escapeErr error // We could embed the text/template field, but it's safer not to because // we need to keep our version of the name space and the underlying // template's in sync. @@ -27,6 +28,9 @@ type Template struct { *nameSpace // common to all associated templates } +// escapeOK is a sentinel value used to indicate valid escaping. +var escapeOK = fmt.Errorf("template escaped correctly") + // nameSpace is the data structure shared by all templates in an association. type nameSpace struct { mu sync.Mutex @@ -51,11 +55,12 @@ func (t *Template) Templates() []*Template { func (t *Template) escape() error { t.nameSpace.mu.Lock() defer t.nameSpace.mu.Unlock() - if !t.escaped { + if t.escapeErr == nil { if err := escapeTemplates(t, t.Name()); err != nil { return err } - t.escaped = true + } else if t.escapeErr != escapeOK { + return t.escapeErr } return nil } @@ -97,13 +102,16 @@ func (t *Template) lookupAndEscapeTemplate(name string) (tmpl *Template, err err if tmpl == nil { return nil, fmt.Errorf("html/template: %q is undefined", name) } + if tmpl.escapeErr != nil && tmpl.escapeErr != escapeOK { + return nil, tmpl.escapeErr + } if tmpl.text.Tree == nil || tmpl.text.Root == nil { return nil, fmt.Errorf("html/template: %q is an incomplete template", name) } if t.text.Lookup(name) == nil { panic("html/template internal error: template escaping out of sync") } - if tmpl != nil && !tmpl.escaped { + if tmpl.escapeErr == nil { err = escapeTemplates(tmpl, name) } return tmpl, err @@ -119,7 +127,7 @@ func (t *Template) lookupAndEscapeTemplate(name string) (tmpl *Template, err err // other than space, comments, and template definitions.) func (t *Template) Parse(src string) (*Template, error) { t.nameSpace.mu.Lock() - t.escaped = false + t.escapeErr = nil t.nameSpace.mu.Unlock() ret, err := t.text.Parse(src) if err != nil { @@ -137,7 +145,7 @@ func (t *Template) Parse(src string) (*Template, error) { tmpl = t.new(name) } // Restore our record of this text/template to its unescaped original state. - tmpl.escaped = false + tmpl.escapeErr = nil tmpl.text = v tmpl.Tree = v.Tree } @@ -151,7 +159,7 @@ func (t *Template) Parse(src string) (*Template, error) { func (t *Template) AddParseTree(name string, tree *parse.Tree) (*Template, error) { t.nameSpace.mu.Lock() defer t.nameSpace.mu.Unlock() - if t.escaped { + if t.escapeErr != nil { return nil, fmt.Errorf("html/template: cannot AddParseTree to %q after it has executed", t.Name()) } text, err := t.text.AddParseTree(name, tree) @@ -159,7 +167,7 @@ func (t *Template) AddParseTree(name string, tree *parse.Tree) (*Template, error return nil, err } ret := &Template{ - false, + nil, text, text.Tree, t.nameSpace, @@ -179,7 +187,7 @@ func (t *Template) AddParseTree(name string, tree *parse.Tree) (*Template, error func (t *Template) Clone() (*Template, error) { t.nameSpace.mu.Lock() defer t.nameSpace.mu.Unlock() - if t.escaped { + if t.escapeErr != nil { return nil, fmt.Errorf("html/template: cannot Clone %q after it has executed", t.Name()) } textClone, err := t.text.Clone() @@ -187,7 +195,7 @@ func (t *Template) Clone() (*Template, error) { return nil, err } ret := &Template{ - false, + nil, textClone, textClone.Tree, &nameSpace{ @@ -197,12 +205,12 @@ func (t *Template) Clone() (*Template, error) { for _, x := range textClone.Templates() { name := x.Name() src := t.set[name] - if src == nil || src.escaped { + if src == nil || src.escapeErr != nil { return nil, fmt.Errorf("html/template: cannot Clone %q after it has executed", t.Name()) } x.Tree = x.Tree.Copy() ret.set[name] = &Template{ - false, + nil, x, x.Tree, ret.nameSpace, @@ -214,7 +222,7 @@ func (t *Template) Clone() (*Template, error) { // New allocates a new HTML template with the given name. func New(name string) *Template { tmpl := &Template{ - false, + nil, template.New(name), nil, &nameSpace{ @@ -237,7 +245,7 @@ func (t *Template) New(name string) *Template { // new is the implementation of New, without the lock. func (t *Template) new(name string) *Template { tmpl := &Template{ - false, + nil, t.text.New(name), nil, t.nameSpace, -- 2.48.1