html/template, text/template: docs and fixes for template redefinition
All prior versions of Go have allowed redefining empty templates
to become non-empty. Unfortunately, that has never consistently
taken effect in html/template after the first execution:
// define and execute
t := template.New("root")
t.Parse(`{{define "T"}}{{end}}<a href="{{template "T"}}">`)
t.Execute(w, nil) // <a href="">
// redefine
t.Parse(`{{define "T"}}my.url{{end}}`) // succeeds, but ignored
t.Execute(w, nil) // <a href="">
When Go 1.6 added {{block...}} to text/template, that loosened the
redefinition rules to allow redefinition at any time. The loosening was
undone a bit in html/template, although inconsistently:
// define and execute
t := template.New("root")
t.Parse(`{{define "T"}}body{{end}}`)
t.Lookup("T").Execute(ioutil.Discard, nil)
// attempt to redefine
t.Parse(`{{define "T"}}body{{end}}`) // rejected in all Go versions
t.Lookup("T").Parse("body") // OK as of Go 1.6, likely unintentionally
Like in the empty->non-empty case, whether future execution takes
notice of a redefinition basically can't be explained without going into
the details of the template escape analysis.
Address both the original inconsistencies in whether a redefinition
would have any effect and the new inconsistencies about whether a
redefinition is allowed by adopting a new rule: no parsing or modifying
any templates after the first execution of any template in the same set.
Template analysis begins at first execution, and once template analysis
has begun, we simply don't have the right logic to update the analysis
for incremental modifications (and never have).
If this new rule breaks existing uses of templates that we decide need
to be supported, we can try to invalidate all escape analysis for the
entire set after any modifications. But let's wait on that until we know
we need to and why.
Also fix documentation of text/template redefinition policy
(redefinition is always OK).
Fixes #15761.
Change-Id: I7d58d7c08a7d9df2440ee0d651a5b2ecaff3006c
Reviewed-on: https://go-review.googlesource.com/31464
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
diff --git a/src/html/template/template_test.go b/src/html/template/template_test.go
index 46df1f8..90c5a73 100644
--- a/src/html/template/template_test.go
+++ b/src/html/template/template_test.go
@@ -1,7 +1,13 @@
-package template
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package template_test
import (
"bytes"
+ . "html/template"
+ "strings"
"testing"
)
@@ -27,3 +33,125 @@
t.Fatalf("got %q; want %q", got, want)
}
}
+
+func TestRedefineNonEmptyAfterExecution(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, `foo`)
+ c.mustExecute(c.root, nil, "foo")
+ c.mustNotParse(c.root, `bar`)
+}
+
+func TestRedefineEmptyAfterExecution(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, ``)
+ c.mustExecute(c.root, nil, "")
+ c.mustNotParse(c.root, `foo`)
+ c.mustExecute(c.root, nil, "")
+}
+
+func TestRedefineAfterNonExecution(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, `{{if .}}<{{template "X"}}>{{end}}{{define "X"}}foo{{end}}`)
+ c.mustExecute(c.root, 0, "")
+ c.mustNotParse(c.root, `{{define "X"}}bar{{end}}`)
+ c.mustExecute(c.root, 1, "<foo>")
+}
+
+func TestRedefineAfterNamedExecution(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, `<{{template "X" .}}>{{define "X"}}foo{{end}}`)
+ c.mustExecute(c.root, nil, "<foo>")
+ c.mustNotParse(c.root, `{{define "X"}}bar{{end}}`)
+ c.mustExecute(c.root, nil, "<foo>")
+}
+
+func TestRedefineNestedByNameAfterExecution(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, `{{define "X"}}foo{{end}}`)
+ c.mustExecute(c.lookup("X"), nil, "foo")
+ c.mustNotParse(c.root, `{{define "X"}}bar{{end}}`)
+ c.mustExecute(c.lookup("X"), nil, "foo")
+}
+
+func TestRedefineNestedByTemplateAfterExecution(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, `{{define "X"}}foo{{end}}`)
+ c.mustExecute(c.lookup("X"), nil, "foo")
+ c.mustNotParse(c.lookup("X"), `bar`)
+ c.mustExecute(c.lookup("X"), nil, "foo")
+}
+
+func TestRedefineSafety(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, `<html><a href="{{template "X"}}">{{define "X"}}{{end}}`)
+ c.mustExecute(c.root, nil, `<html><a href="">`)
+ // Note: Every version of Go prior to Go 1.8 accepted the redefinition of "X"
+ // on the next line, but luckily kept it from being used in the outer template.
+ // Now we reject it, which makes clearer that we're not going to use it.
+ c.mustNotParse(c.root, `{{define "X"}}" bar="baz{{end}}`)
+ c.mustExecute(c.root, nil, `<html><a href="">`)
+}
+
+func TestRedefineTopUse(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, `{{template "X"}}{{.}}{{define "X"}}{{end}}`)
+ c.mustExecute(c.root, 42, `42`)
+ c.mustNotParse(c.root, `{{define "X"}}<script>{{end}}`)
+ c.mustExecute(c.root, 42, `42`)
+}
+
+func TestRedefineOtherParsers(t *testing.T) {
+ c := newTestCase(t)
+ c.mustParse(c.root, ``)
+ c.mustExecute(c.root, nil, ``)
+ if _, err := c.root.ParseFiles("no.template"); err == nil || !strings.Contains(err.Error(), "Execute") {
+ t.Errorf("ParseFiles: %v\nwanted error about already having Executed", err)
+ }
+ if _, err := c.root.ParseGlob("*.no.template"); err == nil || !strings.Contains(err.Error(), "Execute") {
+ t.Errorf("ParseGlob: %v\nwanted error about already having Executed", err)
+ }
+ if _, err := c.root.AddParseTree("t1", c.root.Tree); err == nil || !strings.Contains(err.Error(), "Execute") {
+ t.Errorf("AddParseTree: %v\nwanted error about already having Executed", err)
+ }
+}
+
+type testCase struct {
+ t *testing.T
+ root *Template
+}
+
+func newTestCase(t *testing.T) *testCase {
+ return &testCase{
+ t: t,
+ root: New("root"),
+ }
+}
+
+func (c *testCase) lookup(name string) *Template {
+ return c.root.Lookup(name)
+}
+
+func (c *testCase) mustParse(t *Template, text string) {
+ _, err := t.Parse(text)
+ if err != nil {
+ c.t.Fatalf("parse: %v", err)
+ }
+}
+
+func (c *testCase) mustNotParse(t *Template, text string) {
+ _, err := t.Parse(text)
+ if err == nil {
+ c.t.Fatalf("parse: unexpected success")
+ }
+}
+
+func (c *testCase) mustExecute(t *Template, val interface{}, want string) {
+ var buf bytes.Buffer
+ err := t.Execute(&buf, val)
+ if err != nil {
+ c.t.Fatalf("execute: %v", err)
+ }
+ if buf.String() != want {
+ c.t.Fatalf("template output:\n%s\nwant:\n%s", buf.String(), want)
+ }
+}
