html/template: ignore single untyped nil values
Previous versions of Go like 1.10 did not support using untyped nils as
values in templates very well. For example, using one in a pipeline
resulted in no value being passed, as it was identified as a lack of
arguments.
Since then, we have fixed text/template to fully support untyped nils.
An unforeseen consequence, however, is that html/template now outputs an
escaped "<nil>" when printing an untyped nil, when it used to output
nothing. This is because the template used to skip that value, and it no
longer does in 1.11.
To avoid the change in behavior in 1.11 in the HTML code, make it always
skip a single untyped nil value, just like 1.10. The same is done in
text/template.HTMLEscaper, to keep behavior consistent.
An old and fairly niche html/template test needed updating, as it was
locking in behavior that an untyped nil field should print an escaped
"<nil>". The original fix was for a crash, and html/template has always
ignored untyped nils in all other cases, so it seems safe to bring this
together.
Fixes #25875.
DO NOT SUBMIT - we haven't yet decided if this is the right path
forward.
Change-Id: I34cde7e215ec821e7393300c4742f420c988b511
diff --git a/src/html/template/content.go b/src/html/template/content.go
index 4aadf64..7cda941 100644
--- a/src/html/template/content.go
+++ b/src/html/template/content.go
@@ -147,10 +147,14 @@
}
// stringify converts its arguments to a string and the type of the content.
-// All pointers are dereferenced, as in the text/template package.
+// All pointers are dereferenced, as in the text/template package. Just like in
+// the text/template package, a single untyped nil is ignored to keep backwards
+// compatibility.
func stringify(args ...interface{}) (string, contentType) {
if len(args) == 1 {
switch s := indirect(args[0]).(type) {
+ case nil:
+ return "", contentTypePlain
case string:
return s, contentTypePlain
case CSS:
diff --git a/src/html/template/content_test.go b/src/html/template/content_test.go
index cc092f5..eda3495 100644
--- a/src/html/template/content_test.go
+++ b/src/html/template/content_test.go
@@ -443,17 +443,13 @@
func TestEscapingNilNonemptyInterfaces(t *testing.T) {
tmpl := Must(New("x").Parse("{{.E}}"))
- got := new(bytes.Buffer)
+ buf := new(bytes.Buffer)
testData := struct{ E error }{} // any non-empty interface here will do; error is just ready at hand
- tmpl.Execute(got, testData)
+ tmpl.Execute(buf, testData)
+ got := buf.String()
+ want := ""
- // Use this data instead of just hard-coding "<nil>" to avoid
- // dependencies on the html escaper and the behavior of fmt w.r.t. nil.
- want := new(bytes.Buffer)
- data := struct{ E string }{E: fmt.Sprint(nil)}
- tmpl.Execute(want, data)
-
- if !bytes.Equal(want.Bytes(), got.Bytes()) {
- t.Errorf("expected %q got %q", string(want.Bytes()), string(got.Bytes()))
+ if want != got {
+ t.Errorf("expected %q got %q", want, got)
}
}
diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index d5c258e..2d7d383 100644
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -35,7 +35,8 @@
A, E []string
B, M json.Marshaler
N int
- Z *int
+ U interface{} // untyped nil
+ Z *int // typed nil
W HTML
}{
F: false,
@@ -48,6 +49,7 @@
N: 42,
B: &badMarshaler{},
M: &goodMarshaler{},
+ U: nil,
Z: nil,
W: HTML(`¡<b class="foo">Hello</b>, <textarea>O'World</textarea>!`),
}
@@ -114,6 +116,16 @@
"true",
},
{
+ "nilValueTyped",
+ "{{.Z}}",
+ `<nil>`,
+ },
+ {
+ "nilValueUntyped",
+ "{{.U}}",
+ ``,
+ },
+ {
"constant",
`<a href="/search?q={{"'a<b'"}}">`,
`<a href="/search?q=%27a%3cb%27">`,
@@ -199,11 +211,16 @@
`<button onclick='alert( true )'>`,
},
{
- "jsNilValue",
+ "jsNilValueTyped",
"<button onclick='alert(typeof{{.Z}})'>",
`<button onclick='alert(typeof null )'>`,
},
{
+ "jsNilValueUntyped",
+ "<button onclick='alert(typeof{{.U}})'>",
+ `<button onclick='alert(typeof null )'>`,
+ },
+ {
"jsObjValue",
"<button onclick='alert({{.A}})'>",
`<button onclick='alert(["\u003ca\u003e","\u003cb\u003e"])'>`,
diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
index e54a9ca..7266b43 100644
--- a/src/text/template/exec_test.go
+++ b/src/text/template/exec_test.go
@@ -448,6 +448,8 @@
{"html pipeline", `{{printf "<script>alert(\"XSS\");</script>" | html}}`,
"<script>alert("XSS");</script>", nil, true},
{"html", `{{html .PS}}`, "a string", tVal, true},
+ {"html typed nil", `{{html .NIL}}`, "<nil>", tVal, true},
+ {"html untyped nil", `{{html .Empty0}}`, "", tVal, true},
// JavaScript.
{"js", `{{js .}}`, `It\'d be nice.`, `It'd be nice.`, true},
diff --git a/src/text/template/funcs.go b/src/text/template/funcs.go
index abddfa1..35a9f62 100644
--- a/src/text/template/funcs.go
+++ b/src/text/template/funcs.go
@@ -646,12 +646,20 @@
// fmt.Sprint(args...)
// except that each argument is indirected (if a pointer), as required,
// using the same rules as the default string evaluation during template
-// execution.
+// execution. A single untyped nil argument is also ignored, to keep backwards
+// compatibility with previous Go1 versions.
func evalArgs(args []interface{}) string {
ok := false
var s string
// Fast path for simple common case.
if len(args) == 1 {
+ // Special case for backwards compatibility in Go1. In previous
+ // versions of Go, a single untyped nil argument was ignored by
+ // text/template. This meant that HTML escaping a single untyped
+ // nil value resulted in no output, for example.
+ if args[0] == nil {
+ return ""
+ }
s, ok = args[0].(string)
}
if !ok {
