html/template: prevent aliasing of parse Trees via AddParseTree

Check all associated templates in the set for an existing reference
to the given Tree in AddParseTree before assigning that reference
to a new or existing template. This prevents multiple html/template
Templates from referencing and modifying the same underlying Tree.

While there, fix a few existing unit tests so that they terminate
upon encountering unrecoverable failures.

Fixes #21844

Change-Id: I6b4f6996cf5467113ef94f7b91a6933dbbc21839
Reviewed-on: https://go-review.googlesource.com/64770
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index f5a4ce173644159dcc3702214ff534bb9d007088..92f12ca0e078c2b48c5fb667c7b85b4c9a48b25e 100644 (file)
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -1840,7 +1840,7 @@ func TestErrorOnUndefined(t *testing.T) {
 
        err := tmpl.Execute(nil, nil)
        if err == nil {
-               t.Error("expected error")
+               t.Fatal("expected error")
        }
        if !strings.Contains(err.Error(), "incomplete") {
                t.Errorf("expected error about incomplete template; got %s", err)
@@ -1860,10 +1860,10 @@ func TestIdempotentExecute(t *testing.T) {
        for i := 0; i < 2; i++ {
                err = tmpl.ExecuteTemplate(got, "hello", nil)
                if err != nil {
-                       t.Errorf("unexpected error: %s", err)
+                       t.Fatalf("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)
+                       t.Fatalf("after executing template \"hello\", got:\n\t%q\nwant:\n\t%q\n", got.String(), want)
                }
                got.Reset()
        }
@@ -1871,7 +1871,7 @@ func TestIdempotentExecute(t *testing.T) {
        // "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)
+               t.Fatalf("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 "&amp;amp;".
@@ -1881,6 +1881,19 @@ func TestIdempotentExecute(t *testing.T) {
        }
 }
 
+// This covers issue #21844.
+func TestAddExistingTreeError(t *testing.T) {
+       tmpl := Must(New("foo").Parse(`<p>{{.}}</p>`))
+       tmpl, err := tmpl.AddParseTree("bar", tmpl.Tree)
+       if err == nil {
+               t.Fatalf("expected error after AddParseTree")
+       }
+       const want = `html/template: cannot add parse tree that template "foo" already references`
+       if got := err.Error(); got != want {
+               t.Errorf("got error:\n\t%q\nwant:\n\t%q\n", got, want)
+       }
+}
+
 func BenchmarkEscapedExecute(b *testing.B) {
        tmpl := Must(New("t").Parse(`<a onclick="alert('{{.}}')">{{.}}</a>`))
        var buf bytes.Buffer

diff --git a/src/html/template/template.go b/src/html/template/template.go
index 6a661bf6e5d61e5fc7342fd8f2645fef54a19d04..d77aa3d7df3745352546aecf46a779ce3e72b7d3 100644 (file)
--- a/src/html/template/template.go
+++ b/src/html/template/template.go
@@ -219,6 +219,11 @@ func (t *Template) AddParseTree(name string, tree *parse.Tree) (*Template, error
 
        t.nameSpace.mu.Lock()
        defer t.nameSpace.mu.Unlock()
+       for _, tmpl := range t.set {
+               if tmpl.Tree == tree {
+                       return nil, fmt.Errorf("html/template: cannot add parse tree that template %q already references", tmpl.Name())
+               }
+       }
        text, err := t.text.AddParseTree(name, tree)
        if err != nil {
                return nil, err