Skip to content

Commit

Permalink
html/template: attach functions to namespace
Browse files Browse the repository at this point in the history
The text/template functions are stored in a data structure shared by
all related templates, so do the same with the original, unwrapped,
functions on the html/template side.

For #39807
Fixes #43295

Change-Id: I9f64a0a601f1151c863a2833b5be2baf649b6cef
Reviewed-on: https://go-review.googlesource.com/c/go/+/279492
Trust: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Emmanuel Odeke <[email protected]>
  • Loading branch information
ianlancetaylor committed Jan 7, 2021
1 parent 6da2d3b commit e60cffa
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 19 deletions.
20 changes: 20 additions & 0 deletions src/html/template/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1776,3 +1776,23 @@ func TestRecursiveExecute(t *testing.T) {
t.Fatal(err)
}
}

// Issue 43295.
func TestTemplateFuncsAfterClone(t *testing.T) {
s := `{{ f . }}`
want := "test"
orig := New("orig").Funcs(map[string]interface{}{
"f": func(in string) string {
return in
},
}).New("child")

overviewTmpl := Must(Must(orig.Clone()).Parse(s))
var out strings.Builder
if err := overviewTmpl.Execute(&out, want); err != nil {
t.Fatal(err)
}
if got := out.String(); got != want {
t.Fatalf("got %q; want %q", got, want)
}
}
46 changes: 27 additions & 19 deletions src/html/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ type Template struct {
// template's in sync.
text *template.Template
// The underlying template's parse tree, updated to be HTML-safe.
Tree *parse.Tree
// The original functions, before wrapping.
funcMap FuncMap
Tree *parse.Tree
*nameSpace // common to all associated templates
}

Expand All @@ -42,6 +40,8 @@ type nameSpace struct {
set map[string]*Template
escaped bool
esc escaper
// The original functions, before wrapping.
funcMap FuncMap
}

// Templates returns a slice of the templates associated with t, including t
Expand Down Expand Up @@ -260,7 +260,6 @@ func (t *Template) AddParseTree(name string, tree *parse.Tree) (*Template, error
nil,
text,
text.Tree,
nil,
t.nameSpace,
}
t.set[name] = ret
Expand All @@ -287,14 +286,19 @@ func (t *Template) Clone() (*Template, error) {
}
ns := &nameSpace{set: make(map[string]*Template)}
ns.esc = makeEscaper(ns)
if t.nameSpace.funcMap != nil {
ns.funcMap = make(FuncMap, len(t.nameSpace.funcMap))
for name, fn := range t.nameSpace.funcMap {
ns.funcMap[name] = fn
}
}
wrapFuncs(ns, textClone, ns.funcMap)
ret := &Template{
nil,
textClone,
textClone.Tree,
t.funcMap,
ns,
}
ret.wrapFuncs()
ret.set[ret.Name()] = ret
for _, x := range textClone.Templates() {
name := x.Name()
Expand All @@ -307,10 +311,8 @@ func (t *Template) Clone() (*Template, error) {
nil,
x,
x.Tree,
src.funcMap,
ret.nameSpace,
}
tc.wrapFuncs()
ret.set[name] = tc
}
// Return the template associated with the name of this template.
Expand All @@ -325,7 +327,6 @@ func New(name string) *Template {
nil,
template.New(name),
nil,
nil,
ns,
}
tmpl.set[name] = tmpl
Expand All @@ -351,7 +352,6 @@ func (t *Template) new(name string) *Template {
nil,
t.text.New(name),
nil,
nil,
t.nameSpace,
}
if existing, ok := tmpl.set[name]; ok {
Expand Down Expand Up @@ -382,23 +382,31 @@ type FuncMap map[string]interface{}
// type. However, it is legal to overwrite elements of the map. The return
// value is the template, so calls can be chained.
func (t *Template) Funcs(funcMap FuncMap) *Template {
t.funcMap = funcMap
t.wrapFuncs()
t.nameSpace.mu.Lock()
if t.nameSpace.funcMap == nil {
t.nameSpace.funcMap = make(FuncMap, len(funcMap))
}
for name, fn := range funcMap {
t.nameSpace.funcMap[name] = fn
}
t.nameSpace.mu.Unlock()

wrapFuncs(t.nameSpace, t.text, funcMap)
return t
}

// wrapFuncs records the functions with text/template. We wrap them to
// unlock the nameSpace. See TestRecursiveExecute for a test case.
func (t *Template) wrapFuncs() {
if len(t.funcMap) == 0 {
func wrapFuncs(ns *nameSpace, textTemplate *template.Template, funcMap FuncMap) {
if len(funcMap) == 0 {
return
}
tfuncs := make(template.FuncMap, len(t.funcMap))
for name, fn := range t.funcMap {
tfuncs := make(template.FuncMap, len(funcMap))
for name, fn := range funcMap {
fnv := reflect.ValueOf(fn)
wrapper := func(args []reflect.Value) []reflect.Value {
t.nameSpace.mu.RUnlock()
defer t.nameSpace.mu.RLock()
ns.mu.RUnlock()
defer ns.mu.RLock()
if fnv.Type().IsVariadic() {
return fnv.CallSlice(args)
} else {
Expand All @@ -408,7 +416,7 @@ func (t *Template) wrapFuncs() {
wrapped := reflect.MakeFunc(fnv.Type(), wrapper)
tfuncs[name] = wrapped.Interface()
}
t.text.Funcs(tfuncs)
textTemplate.Funcs(tfuncs)
}

// Delims sets the action delimiters to the specified strings, to be used in
Expand Down

0 comments on commit e60cffa

Please sign in to comment.