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>

diff --git a/src/html/template/content.go b/src/html/template/content.go
index 4aadf64df252f2e1521916e13a38174ef7793098..6ba87a955035abc49eb3136bdc2eee7537f5aba6 100644 (file)
--- 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 cc092f50c0c1e4df2f36a8f32fdcb40191b44633..72d56f50c14413ada8cace52f043afea819b0add 100644 (file)
--- 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 "<nil>" 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 35d171c3fca2d82b1bab8e7b1cfcc2f963ce41d8..290ec81b9672a2f9910aa406662d4193d4339cac 100644 (file)
--- 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 d5c258ecaa87d0da6e8aa283a26489550940603c..e6c12a8a254b2bded9e565c213458851b617e4c2 100644 (file)
--- 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>!`),
        }
@@ -113,6 +115,16 @@ func TestEscape(t *testing.T) {
                        "{{.T}}",
                        "true",
                },
+               {
+                       "untypedNilValue",
+                       "{{.U}}",
+                       "",
+               },
+               {
+                       "typedNilValue",
+                       "{{.Z}}",
+                       "&lt;nil&gt;",
+               },
                {
                        "constant",
                        `<a href="/search?q={{"'a<b'"}}">`,
@@ -199,10 +211,15 @@ 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}})'>",

diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
index e54a9ca3c792e2409dd22632307c14a574c5ce32..6f40d80635fd0c691e6d6bf2291a7ccedeaf6f17 100644 (file)
--- a/src/text/template/exec_test.go
+++ b/src/text/template/exec_test.go
@@ -448,6 +448,8 @@ var execTests = []execTest{
        {"html pipeline", `{{printf "<script>alert(\"XSS\");</script>" | html}}`,
                "&lt;script&gt;alert(&#34;XSS&#34;);&lt;/script&gt;", nil, true},
        {"html", `{{html .PS}}`, "a string", tVal, true},
+       {"html typed nil", `{{html .NIL}}`, "&lt;nil&gt;", tVal, true},
+       {"html untyped nil", `{{html .Empty0}}`, "&lt;no value&gt;", tVal, true},
 
        // JavaScript.
        {"js", `{{js .}}`, `It\'d be nice.`, `It'd be nice.`, true},