html/template: allow safe usage of predefined escapers in pipelines

Allow the predefined escapers "html", "urlquery", and "js" to be used
in pipelines when they have no potential to affect the correctness or
safety of the escaped pipeline output. Specifically:
- "urlquery" may be used if it is the last command in the pipeline.
- "html" may be used if it is the last command in the pipeline, and
  the pipeline does not occur in an unquoted HTML attribute value
  context.
- "js" may be used in any pipeline, since it does not affect the
  merging of contextual escapers.

This change will loosens the restrictions on predefined escapers
introduced in golang.org/cl/37880, which will hopefully ease the
upgrade path for existing template users.

This change brings back the escaper-merging logic, and associated
unit tests, that were removed in golang.org/cl/37880. However, a
few notable changes have been made:
- "_html_template_nospaceescaper" is no longer considered
  equivalent to "html", since the former escapes spaces, while
  the latter does not (see #19345). This change should not silently
  break any templates, since pipelines where this substituion will
  happen will already trigger an explicit error.
- An "_eval_args_" internal directive has been added to
  handle pipelines containing a single explicit call to a
  predefined escaper, e.g. {{html .X}} (see #19353).

Also, the HTMLEscape function called by the predefined
text/template "html" function now escapes the NULL character as
well. This effectively makes it as secure as the internal
html/template HTML escapers (see #19345). While this change is
backward-incompatible, it will only affect illegitimate uses
of this escaper, since the NULL character is always illegal in
valid HTML.

Fixes #19952

Change-Id: I9b5570a80a3ea284b53901e6a1f842fc59b33d3a
Reviewed-on: https://go-review.googlesource.com/40936
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

1 files changed, 17 insertions, 10 deletions

diff --git a/src/html/template/error.go b/src/html/template/error.go
index 3b70ba1ec8..0e527063ea 100644
--- a/src/html/template/error.go
+++ b/src/html/template/error.go
@@ -186,23 +186,30 @@ const (
 
 	// ErrPredefinedEscaper: "predefined escaper ... disallowed in template"
 	// Example:
-	//   <a href="{{.X | urlquery}}">
+	//   <div class={{. | html}}>Hello<div>
 	// Discussion:
 	//   Package html/template already contextually escapes all pipelines to
 	//   produce HTML output safe against code injection. Manually escaping
-	//   pipeline output using the predefined escapers "html", "urlquery", or "js"
-	//   is unnecessary, and might affect the correctness or safety of the escaped
-	//   pipeline output. In the above example, "urlquery" should simply be
-	//   removed from the pipeline so that escaping is performed solely by the
-	//   contextual autoescaper.
-	//   If the predefined escaper occurs in the middle of a pipeline where
-	//   subsequent commands expect escaped input, e.g.
+	//   pipeline output using the predefined escapers "html" or "urlquery" is
+	//   unnecessary, and may affect the correctness or safety of the escaped
+	//   pipeline output in Go 1.8 and earlier.
+	//
+	//   In most cases, such as the given example, this error can be resolved by
+	//   simply removing the predefined escaper from the pipeline and letting the
+	//   contextual autoescaper handle the escaping of the pipeline. In other
+	//   instances, where the predefined escaper occurs in the middle of a
+	//   pipeline where subsequent commands expect escaped input, e.g.
 	//     {{.X | html | makeALink}}
 	//   where makeALink does
-	//     return "<a href='+input+'>link</a>"
+	//     return `<a href="`+input+`">link</a>`
 	//   consider refactoring the surrounding template to make use of the
 	//   contextual autoescaper, i.e.
-	//     <a href='{{.X}}'>link</a>
+	//     <a href="{{.X}}">link</a>
+	//
+	//   To ease migration to Go 1.9 and beyond, "html" and "urlquery" will
+	//   continue to be allowed as the last command in a pipeline. However, if the
+	//   pipeline occurs in an unquoted attribute value context, "html" is
+	//   disallowed. Avoid using "html" and "urlquery" entirely in new templates.
 	ErrPredefinedEscaper
 )