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

diff --git a/src/pkg/html/template/escape.go b/src/pkg/html/template/escape.go
index 4e379828d4c33b5cfd86083a4e5c2a85ddccaad5..3ba3747f6ff011496c4fe9df1f95d4e0faaea84c 100644 (file)
--- 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 3ccf93ece01877a2563f47702ad83f7bb919eefc..5509b836f9eaab772715df82105cb7bcf2e22e4f 100644 (file)
--- 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 d389658979aa4c045fa7a291c999340dd9a2aad3..538837cc5c403850c54352b49ffd44413fc2dfce 100644 (file)
--- 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,