text/template: fix race condition on function maps

The Template objects are supposed to be goroutine-safe once they
have been parsed. This includes the text and html ones.

For html/template, the escape mechanism is triggered at execution
time. It may alter the internal structures of the template, so
a mutex protects them against concurrent accesses.

The text/template package is free of any synchronization primitive.

A race condition may occur when nested templates are escaped:
the escape algorithm alters the function maps of the associated
text templates, while a concurrent template execution may access
the function maps in read mode.

The less invasive fix I have found is to introduce a RWMutex in
text/template to protect the function maps. This is unfortunate
but it should be effective.

Fixes #9945

Change-Id: I1edb73c0ed0f1fcddd2f1516230b548b92ab1269
Reviewed-on: https://go-review.googlesource.com/10101
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/text/template/funcs.go b/src/text/template/funcs.go
index cdd187bda248b2b655d0ead71cd8688fa51d64f2..ccd0dfc80d86b024a9759dddcdf82520cffc680b 100644 (file)
--- a/src/text/template/funcs.go
+++ b/src/text/template/funcs.go
@@ -92,6 +92,8 @@ func goodFunc(typ reflect.Type) bool {
 // findFunction looks for a function in the template, and global map.
 func findFunction(name string, tmpl *Template) (reflect.Value, bool) {
        if tmpl != nil && tmpl.common != nil {
+               tmpl.muFuncs.RLock()
+               defer tmpl.muFuncs.RUnlock()
                if fn := tmpl.execFuncs[name]; fn.IsValid() {
                        return fn, true
                }

diff --git a/src/text/template/template.go b/src/text/template/template.go
index 8611faad9f2838c00d1cbf16c7881fe5863a5615..a7c5c8cd2c13fb971b27f73ca8ce90d4846ad660 100644 (file)
--- a/src/text/template/template.go
+++ b/src/text/template/template.go
@@ -7,18 +7,20 @@ package template
 import (
        "fmt"
        "reflect"
+       "sync"
        "text/template/parse"
 )
 
 // common holds the information shared by related templates.
 type common struct {
-       tmpl map[string]*Template
+       tmpl   map[string]*Template
+       option option
        // We use two maps, one for parsing and one for execution.
        // This separation makes the API cleaner since it doesn't
        // expose reflection to the client.
+       muFuncs    sync.RWMutex // protects parseFuncs and execFuncs
        parseFuncs FuncMap
        execFuncs  map[string]reflect.Value
-       option     option
 }
 
 // Template is the representation of a parsed template. The *parse.Tree
@@ -84,6 +86,8 @@ func (t *Template) Clone() (*Template, error) {
                tmpl := v.copy(nt.common)
                nt.tmpl[k] = tmpl
        }
+       t.muFuncs.RLock()
+       defer t.muFuncs.RUnlock()
        for k, v := range t.parseFuncs {
                nt.parseFuncs[k] = v
        }
@@ -146,6 +150,8 @@ func (t *Template) Delims(left, right string) *Template {
 // value is the template, so calls can be chained.
 func (t *Template) Funcs(funcMap FuncMap) *Template {
        t.init()
+       t.muFuncs.Lock()
+       defer t.muFuncs.Unlock()
        addValueFuncs(t.execFuncs, funcMap)
        addFuncs(t.parseFuncs, funcMap)
        return t
@@ -169,7 +175,9 @@ func (t *Template) Lookup(name string) *Template {
 // can contain text other than space, comments, and template definitions.)
 func (t *Template) Parse(text string) (*Template, error) {
        t.init()
+       t.muFuncs.RLock()
        trees, err := parse.Parse(t.name, text, t.leftDelim, t.rightDelim, t.parseFuncs, builtins)
+       t.muFuncs.RUnlock()
        if err != nil {
                return nil, err
        }