html/template: ignore untyped nil arguments to default escapers

CL 95215 changed text/template so that untyped nil arguments were no
longer ignored, but were instead passed to functions as expected.
This had an unexpected effect on html/template, where all data is
implicitly passed to functions: originally untyped nil arguments were
not passed and were thus effectively ignored, but after CL 95215 they
were passed and were printed, typically as an escaped version of "<nil>".

This CL restores some of the behavior of html/template by ignoring
untyped nil arguments passed implicitly to escaper functions.

While eliminating one change to html/template relative to earlier
releases, this unfortunately introduces a different one: originally
values of interface type with the value nil were printed as an escaped
version of "<nil>". With this CL they are ignored as though they were
untyped nil values. My judgement is that this is a less common case.
We'll see.

This CL adds some tests of typed and untyped nil values to
html/template and text/template to capture the current behavior.

Updates #18716
Fixes #25875

Change-Id: I5912983ca32b31ece29e929e72d503b54d7b0cac
Reviewed-on: https://go-review.googlesource.com/121815
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

4 files changed, 35 insertions, 7 deletions

diff --git a/src/html/template/content.go b/src/html/template/content.go
index 4aadf64df2..6ba87a9550 100644
--- a/src/html/template/content.go
+++ b/src/html/template/content.go
@@ -169,8 +169,17 @@ func stringify(args ...interface{}) (string, contentType) {
 			return string(s), contentTypeSrcset
 		}
 	}
-	for i, arg := range args {
+	i := 0
+	for _, arg := range args {
+		// We skip untyped nil arguments for backward compatibility.
+		// Without this they would be output as <nil>, escaped.
+		// See issue 25875.
+		if arg == nil {
+			continue
+		}
+
 		args[i] = indirectToStringerOrError(arg)
+		i++
 	}
-	return fmt.Sprint(args...), contentTypePlain
+	return fmt.Sprint(args[:i]...), contentTypePlain
 }
diff --git a/src/html/template/content_test.go b/src/html/template/content_test.go
index cc092f50c0..72d56f50c1 100644
--- a/src/html/template/content_test.go
+++ b/src/html/template/content_test.go
@@ -447,10 +447,9 @@ func TestEscapingNilNonemptyInterfaces(t *testing.T) {
 	testData := struct{ E error }{} // any non-empty interface here will do; error is just ready at hand
 	tmpl.Execute(got, testData)
 
-	// Use this data instead of just hard-coding "&lt;nil&gt;" to avoid
-	// dependencies on the html escaper and the behavior of fmt w.r.t. nil.
+	// A non-empty interface should print like an empty interface.
 	want := new(bytes.Buffer)
-	data := struct{ E string }{E: fmt.Sprint(nil)}
+	data := struct{ E interface{} }{}
 	tmpl.Execute(want, data)
 
 	if !bytes.Equal(want.Bytes(), got.Bytes()) {
diff --git a/src/html/template/doc.go b/src/html/template/doc.go
index 35d171c3fc..290ec81b96 100644
--- a/src/html/template/doc.go
+++ b/src/html/template/doc.go
@@ -70,6 +70,9 @@ In this case it becomes
 where urlescaper, attrescaper, and htmlescaper are aliases for internal escaping
 functions.
 
+For these internal escaping functions, if an action pipeline evaluates to
+a nil interface value, it is treated as though it were an empty string.
+
 Errors
 
 See the documentation of ErrorCode for details.
diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go
index d5c258ecaa..e6c12a8a25 100644
--- a/src/html/template/escape_test.go
+++ b/src/html/template/escape_test.go
@@ -35,7 +35,8 @@ func TestEscape(t *testing.T) {
 		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 @@ func TestEscape(t *testing.T) {
 		N: 42,
 		B: &badMarshaler{},
 		M: &goodMarshaler{},
+		U: nil,
 		Z: nil,
 		W: HTML(`&iexcl;<b class="foo">Hello</b>, <textarea>O'World</textarea>!`),
 	}
@@ -114,6 +116,16 @@ func TestEscape(t *testing.T) {
 			"true",
 		},
 		{
+			"untypedNilValue",
+			"{{.U}}",
+			"",
+		},
+		{
+			"typedNilValue",
+			"{{.Z}}",
+			"&lt;nil&gt;",
+		},
+		{
 			"constant",
 			`<a href="/search?q={{"'a<b'"}}">`,
 			`<a href="/search?q=%27a%3cb%27">`,
@@ -199,11 +211,16 @@ func TestEscape(t *testing.T) {
 			`<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([&#34;\u003ca\u003e&#34;,&#34;\u003cb\u003e&#34;])'>`,