text/template/parse, html/template: copy Tree.text during html template clone

The root cause of the panic reported in https://code.google.com/p/go/issues/detail?id=5980
is that parse's Tree.Text wasn't being copied during the clone.

Fix this by adding and using a Copy method for parse.Tree.

Fixes #5980.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/12420044

diff --git a/src/pkg/html/template/clone_test.go b/src/pkg/html/template/clone_test.go
index 2663cddc24b84d3e892874c5184bec263d08667c..e11bff2c5dfa7bf7c4dc20e3b16fde484f1ce82b 100644 (file)
--- a/src/pkg/html/template/clone_test.go
+++ b/src/pkg/html/template/clone_test.go
@@ -6,6 +6,8 @@ package template
 
 import (
        "bytes"
+       "errors"
+       "io/ioutil"
        "testing"
        "text/template/parse"
 )
@@ -146,3 +148,41 @@ func TestCloneCrash(t *testing.T) {
        Must(t1.New("t1").Parse(`{{define "foo"}}foo{{end}}`))
        t1.Clone()
 }
+
+// Ensure that this guarantee from the docs is upheld:
+// "Further calls to Parse in the copy will add templates
+// to the copy but not to the original."
+func TestCloneThenParse(t *testing.T) {
+       t0 := Must(New("t0").Parse(`{{define "a"}}{{template "embedded"}}{{end}}`))
+       t1 := Must(t0.Clone())
+       Must(t1.Parse(`{{define "embedded"}}t1{{end}}`))
+       if len(t0.Templates())+1 != len(t1.Templates()) {
+               t.Error("adding a template to a clone added it to the original")
+       }
+       // double check that the embedded template isn't available in the original
+       err := t0.ExecuteTemplate(ioutil.Discard, "a", nil)
+       if err == nil {
+               t.Error("expected 'no such template' error")
+       }
+}
+
+// https://code.google.com/p/go/issues/detail?id=5980
+func TestFuncMapWorksAfterClone(t *testing.T) {
+       funcs := FuncMap{"customFunc": func() (string, error) {
+               return "", errors.New("issue5980")
+       }}
+
+       // get the expected error output (no clone)
+       uncloned := Must(New("").Funcs(funcs).Parse("{{customFunc}}"))
+       wantErr := uncloned.Execute(ioutil.Discard, nil)
+
+       // toClone must be the same as uncloned. It has to be recreated from scratch,
+       // since cloning cannot occur after execution.
+       toClone := Must(New("").Funcs(funcs).Parse("{{customFunc}}"))
+       cloned := Must(toClone.Clone())
+       gotErr := cloned.Execute(ioutil.Discard, nil)
+
+       if wantErr.Error() != gotErr.Error() {
+               t.Errorf("clone error message mismatch want %q got %q", wantErr, gotErr)
+       }
+}

diff --git a/src/pkg/html/template/template.go b/src/pkg/html/template/template.go
index 5862f01f45e374562c6f0e95db9b67568d87d6c4..db7244e4249e8dc084d3386ef8114836f76253b3 100644 (file)
--- a/src/pkg/html/template/template.go
+++ b/src/pkg/html/template/template.go
@@ -190,12 +190,7 @@ func (t *Template) Clone() (*Template, error) {
                if src == nil || src.escaped {
                        return nil, fmt.Errorf("html/template: cannot Clone %q after it has executed", t.Name())
                }
-               if x.Tree != nil {
-                       x.Tree = &parse.Tree{
-                               Name: x.Tree.Name,
-                               Root: x.Tree.Root.CopyList(),
-                       }
-               }
+               x.Tree = x.Tree.Copy()
                ret.set[name] = &Template{
                        false,
                        x,

diff --git a/src/pkg/text/template/parse/parse.go b/src/pkg/text/template/parse/parse.go
index be83e77cf54e87f66d0b59101e622180f9b3153b..34112fb7b3547ce95338d7a6ee3df69eab313497 100644 (file)
--- a/src/pkg/text/template/parse/parse.go
+++ b/src/pkg/text/template/parse/parse.go
@@ -30,6 +30,19 @@ type Tree struct {
        vars      []string // variables defined at the moment.
 }
 
+// Copy returns a copy of the Tree. Any parsing state is discarded.
+func (t *Tree) Copy() *Tree {
+       if t == nil {
+               return nil
+       }
+       return &Tree{
+               Name:      t.Name,
+               ParseName: t.ParseName,
+               Root:      t.Root.CopyList(),
+               text:      t.text,
+       }
+}
+
 // Parse returns a map from template name to parse.Tree, created by parsing the
 // templates described in the argument string. The top-level template will be
 // given the specified name. If an error is encountered, parsing stops and an

diff --git a/src/pkg/text/template/parse/parse_test.go b/src/pkg/text/template/parse/parse_test.go
index 049e65c7d34c55528af5ca002b463c45affa7599..ba1a18ec5426b338e02a8a3b26822c8649f54cc1 100644 (file)
--- a/src/pkg/text/template/parse/parse_test.go
+++ b/src/pkg/text/template/parse/parse_test.go
@@ -332,6 +332,22 @@ func TestIsEmpty(t *testing.T) {
        }
 }
 
+func TestErrorContextWithTreeCopy(t *testing.T) {
+       tree, err := New("root").Parse("{{if true}}{{end}}", "", "", make(map[string]*Tree), nil)
+       if err != nil {
+               t.Fatalf("unexpected tree parse failure: %v", err)
+       }
+       treeCopy := tree.Copy()
+       wantLocation, wantContext := tree.ErrorContext(tree.Root.Nodes[0])
+       gotLocation, gotContext := treeCopy.ErrorContext(treeCopy.Root.Nodes[0])
+       if wantLocation != gotLocation {
+               t.Errorf("wrong error location want %q got %q", wantLocation, gotLocation)
+       }
+       if wantContext != gotContext {
+               t.Errorf("wrong error location want %q got %q", wantContext, gotContext)
+       }
+}
+
 // All failures, and the result is a string that must appear in the error message.
 var errorTests = []parseTest{
        // Check line numbers are accurate.