From c5cb4843e174697dd060b42810b8d20f0998b2e6 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Fri, 29 Jun 2018 17:31:37 -0700 Subject: [PATCH] html/template: ignore untyped nil arguments to default escapers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 "". 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 "". 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 Reviewed-by: Daniel Martí Reviewed-by: Russ Cox TryBot-Result: Gobot Gobot --- src/html/template/content.go | 13 +++++++++++-- src/html/template/content_test.go | 5 ++--- src/html/template/doc.go | 3 +++ src/html/template/escape_test.go | 21 +++++++++++++++++++-- src/text/template/exec_test.go | 2 ++ 5 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/html/template/content.go b/src/html/template/content.go index 4aadf64df25..6ba87a95503 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 , 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 cc092f50c0c..72d56f50c14 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 "<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 35d171c3fca..290ec81b967 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 d5c258ecaa8..e6c12a8a254 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(`¡Hello, !`), } @@ -113,6 +115,16 @@ func TestEscape(t *testing.T) { "{{.T}}", "true", }, + { + "untypedNilValue", + "{{.U}}", + "", + }, + { + "typedNilValue", + "{{.Z}}", + "<nil>", + }, { "constant", ``, @@ -199,10 +211,15 @@ func TestEscape(t *testing.T) { `