text/template: verify that names in FuncMap are valid identifiers

There was no verification in Funcs that the map had valid names,
which meant that the error could only be caught when parsing
the template that tried to use them. Fix this by validating the names
in Funcs and panicking before parsing if there is a bad name.

This is arguably an API change, since it didn't trigger a panic
before, but Funcs did already panic if the function itself was no
good, so I argue it's an acceptable change to add more sanity
checks.

Fixes #9685.

Change-Id: Iabf1d0602c49d830f3ed71ca1ccc7eb9a5521ff5
Reviewed-on: https://go-review.googlesource.com/14562
Reviewed-by: Andrew Gerrand <adg@golang.org>

1 files changed, 49 insertions, 0 deletions

diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
index b2ed8e7938..139fc5320d 100644
--- a/src/text/template/exec_test.go
+++ b/src/text/template/exec_test.go
@@ -1183,3 +1183,52 @@ func TestExecuteGivesExecError(t *testing.T) {
 		t.Errorf("expected %q; got %q", expect, err)
 	}
 }
+
+func funcNameTestFunc() int {
+	return 0
+}
+
+func TestGoodFuncNames(t *testing.T) {
+	names := []string{
+		"_",
+		"a",
+		"a1",
+		"a1",
+		"Ӵ",
+	}
+	for _, name := range names {
+		tmpl := New("X").Funcs(
+			FuncMap{
+				name: funcNameTestFunc,
+			},
+		)
+		if tmpl == nil {
+			t.Fatalf("nil result for %q", name)
+		}
+	}
+}
+
+func TestBadFuncNames(t *testing.T) {
+	names := []string{
+		"",
+		"2",
+		"a-b",
+	}
+	for _, name := range names {
+		testBadFuncName(name, t)
+	}
+}
+
+func testBadFuncName(name string, t *testing.T) {
+	defer func() {
+		recover()
+	}()
+	New("X").Funcs(
+		FuncMap{
+			name: funcNameTestFunc,
+		},
+	)
+	// If we get here, the name did not cause a panic, which is how Funcs
+	// reports an error.
+	t.Errorf("%q succeeded incorrectly as function name", name)
+}