Skip to content

Commit

Permalink
Fix corruption in templates that use title function (prometheus#3278)
Browse files Browse the repository at this point in the history
This commit fixes data corruption in templates that use the title
function as it used a shared cases.Title when casers should not
be shared between goroutines. When templates that used the title
function were executed at the same time, data corruption would occur
in the text that was being cased. This is fixed using a separate
caser for each function call.

Add tests to assertDefaultFuncs are thread-safe

Signed-off-by: George Robinson <[email protected]>
  • Loading branch information
grobinson-grafana authored and hoperays committed Apr 23, 2023
1 parent 4044cb3 commit 25fd54e
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 4 deletions.
11 changes: 7 additions & 4 deletions template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,14 @@ func (t *Template) ExecuteHTMLString(html string, data interface{}) (string, err
type FuncMap map[string]interface{}

var DefaultFuncs = FuncMap{
"toUpper": strings.ToUpper,
"toLower": strings.ToLower,
"toUpper": strings.ToUpper,
"toLower": strings.ToLower,
"title": func(text string) string {
// casers should not be shared between goroutines, instead
// create a new caser each time this function is called
return cases.Title(language.AmericanEnglish).String(text)
},
"trimSpace": strings.TrimSpace,

"title": cases.Title(language.AmericanEnglish).String,
// join is equal to strings.Join but inverts the argument order
// for easier pipelining in templates.
"join": func(sep string, s []string) string {
Expand Down
58 changes: 58 additions & 0 deletions template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package template
import (
tmplhtml "html/template"
"net/url"
"sync"
"testing"
tmpltext "text/template"
"time"
Expand Down Expand Up @@ -465,3 +466,60 @@ func TestTemplateExpansionWithOptions(t *testing.T) {
})
}
}

// This test asserts that template functions are thread-safe.
func TestTemplateFuncs(t *testing.T) {
tmpl, err := FromGlobs([]string{})
require.NoError(t, err)

for _, tc := range []struct {
title string
in string
data interface{}
exp string
}{{
title: "Template using toUpper",
in: `{{ "abc" | toUpper }}`,
exp: "ABC",
}, {
title: "Template using toLower",
in: `{{ "ABC" | toLower }}`,
exp: "abc",
}, {
title: "Template using title",
in: `{{ "abc" | title }}`,
exp: "Abc",
}, {
title: "Template using trimSpace",
in: `{{ " abc " | trimSpace }}`,
exp: "abc",
}, {
title: "Template using join",
in: `{{ . | join "," }}`,
data: []string{"abc", "def"},
exp: "abc,def",
}, {
title: "Template using match",
in: `{{ match "[a-z]+" "abc" }}`,
exp: "true",
}, {
title: "Template using reReplaceAll",
in: `{{ reReplaceAll "ab" "AB" "abc" }}`,
exp: "ABc",
}} {
tc := tc
t.Run(tc.title, func(t *testing.T) {
wg := sync.WaitGroup{}
for i := 0; i < 10; i++ {
wg.Add(1)
go func() {
defer wg.Done()
got, err := tmpl.ExecuteTextString(tc.in, tc.data)
require.NoError(t, err)
require.Equal(t, tc.exp, got)
}()
}
wg.Wait()
})
}
}

0 comments on commit 25fd54e

Please sign in to comment.