Skip to content

Commit

Permalink
html/template: support parsing complex JS template literals
Browse files Browse the repository at this point in the history
This change undoes the restrictions added in CL 482079, which added a
blanket ban on using actions within JS template literal strings, and
adds logic to support actions while properly applies contextual escaping
based on the correct context within the literal.

Since template literals can contain both normal strings, and nested JS
contexts, logic is required to properly track those context switches
during parsing.

ErrJsTmplLit is deprecated, and the GODEBUG flag jstmpllitinterp no
longer does anything.

Fixes #61619

Change-Id: I0338cc6f663723267b8f7aaacc55aa28f60906f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/507995
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Roland Shoemaker <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
  • Loading branch information
rolandshoemaker authored and gopherbot committed Oct 2, 2023
1 parent deb8e29 commit c9c885f
Show file tree
Hide file tree
Showing 8 changed files with 253 additions and 58 deletions.
1 change: 1 addition & 0 deletions api/next/61619.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pkg html/template, const ErrJSTemplate //deprecated #61619
24 changes: 13 additions & 11 deletions src/html/template/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,16 @@ import (
// https://www.w3.org/TR/html5/syntax.html#the-end
// where the context element is null.
type context struct {
state state
delim delim
urlPart urlPart
jsCtx jsCtx
attr attr
element element
n parse.Node // for range break/continue
err *Error
state state
delim delim
urlPart urlPart
jsCtx jsCtx
jsTmplExprDepth int
jsBraceDepth int
attr attr
element element
n parse.Node // for range break/continue
err *Error
}

func (c context) String() string {
Expand Down Expand Up @@ -120,8 +122,8 @@ const (
stateJSDqStr
// stateJSSqStr occurs inside a JavaScript single quoted string.
stateJSSqStr
// stateJSBqStr occurs inside a JavaScript back quoted string.
stateJSBqStr
// stateJSTmplLit occurs inside a JavaScript back quoted string.
stateJSTmplLit
// stateJSRegexp occurs inside a JavaScript regexp literal.
stateJSRegexp
// stateJSBlockCmt occurs inside a JavaScript /* block comment */.
Expand Down Expand Up @@ -182,7 +184,7 @@ func isInScriptLiteral(s state) bool {
// stateJSHTMLOpenCmt, stateJSHTMLCloseCmt) because their content is already
// omitted from the output.
switch s {
case stateJSDqStr, stateJSSqStr, stateJSBqStr, stateJSRegexp:
case stateJSDqStr, stateJSSqStr, stateJSTmplLit, stateJSRegexp:
return true
}
return false
Expand Down
4 changes: 4 additions & 0 deletions src/html/template/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,10 @@ const (
// Discussion:
// Package html/template does not support actions inside of JS template
// literals.
//
// Deprecated: ErrJSTemplate is no longer returned when an action is present
// in a JS template literal. Actions inside of JS template literals are now
// escaped as expected.
ErrJSTemplate
)

Expand Down
48 changes: 22 additions & 26 deletions src/html/template/escape.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,23 @@ func evalArgs(args ...any) string {

// funcMap maps command names to functions that render their inputs safe.
var funcMap = template.FuncMap{
"_html_template_attrescaper": attrEscaper,
"_html_template_commentescaper": commentEscaper,
"_html_template_cssescaper": cssEscaper,
"_html_template_cssvaluefilter": cssValueFilter,
"_html_template_htmlnamefilter": htmlNameFilter,
"_html_template_htmlescaper": htmlEscaper,
"_html_template_jsregexpescaper": jsRegexpEscaper,
"_html_template_jsstrescaper": jsStrEscaper,
"_html_template_jsvalescaper": jsValEscaper,
"_html_template_nospaceescaper": htmlNospaceEscaper,
"_html_template_rcdataescaper": rcdataEscaper,
"_html_template_srcsetescaper": srcsetFilterAndEscaper,
"_html_template_urlescaper": urlEscaper,
"_html_template_urlfilter": urlFilter,
"_html_template_urlnormalizer": urlNormalizer,
"_eval_args_": evalArgs,
"_html_template_attrescaper": attrEscaper,
"_html_template_commentescaper": commentEscaper,
"_html_template_cssescaper": cssEscaper,
"_html_template_cssvaluefilter": cssValueFilter,
"_html_template_htmlnamefilter": htmlNameFilter,
"_html_template_htmlescaper": htmlEscaper,
"_html_template_jsregexpescaper": jsRegexpEscaper,
"_html_template_jsstrescaper": jsStrEscaper,
"_html_template_jstmpllitescaper": jsTmplLitEscaper,
"_html_template_jsvalescaper": jsValEscaper,
"_html_template_nospaceescaper": htmlNospaceEscaper,
"_html_template_rcdataescaper": rcdataEscaper,
"_html_template_srcsetescaper": srcsetFilterAndEscaper,
"_html_template_urlescaper": urlEscaper,
"_html_template_urlfilter": urlFilter,
"_html_template_urlnormalizer": urlNormalizer,
"_eval_args_": evalArgs,
}

// escaper collects type inferences about templates and changes needed to make
Expand Down Expand Up @@ -227,16 +228,8 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
c.jsCtx = jsCtxDivOp
case stateJSDqStr, stateJSSqStr:
s = append(s, "_html_template_jsstrescaper")
case stateJSBqStr:
if debugAllowActionJSTmpl.Value() == "1" {
debugAllowActionJSTmpl.IncNonDefault()
s = append(s, "_html_template_jsstrescaper")
} else {
return context{
state: stateError,
err: errorf(ErrJSTemplate, n, n.Line, "%s appears in a JS template literal", n),
}
}
case stateJSTmplLit:
s = append(s, "_html_template_jstmpllitescaper")
case stateJSRegexp:
s = append(s, "_html_template_jsregexpescaper")
case stateCSS:
Expand Down Expand Up @@ -395,6 +388,9 @@ var redundantFuncs = map[string]map[string]bool{
"_html_template_jsstrescaper": {
"_html_template_attrescaper": true,
},
"_html_template_jstmpllitescaper": {
"_html_template_attrescaper": true,
},
"_html_template_urlescaper": {
"_html_template_urlnormalizer": true,
},
Expand Down
135 changes: 122 additions & 13 deletions src/html/template/escape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ func (x *goodMarshaler) MarshalJSON() ([]byte, error) {

func TestEscape(t *testing.T) {
data := struct {
F, T bool
C, G, H string
A, E []string
B, M json.Marshaler
N int
U any // untyped nil
Z *int // typed nil
W HTML
F, T bool
C, G, H, I string
A, E []string
B, M json.Marshaler
N int
U any // untyped nil
Z *int // typed nil
W HTML
}{
F: false,
T: true,
Expand All @@ -52,6 +52,7 @@ func TestEscape(t *testing.T) {
U: nil,
Z: nil,
W: HTML(`&iexcl;<b class="foo">Hello</b>, <textarea>O'World</textarea>!`),
I: "${ asd `` }",
}
pdata := &data

Expand Down Expand Up @@ -718,6 +719,21 @@ func TestEscape(t *testing.T) {
"<p name=\"{{.U}}\">",
"<p name=\"\">",
},
{
"JS template lit special characters",
"<script>var a = `{{.I}}`</script>",
"<script>var a = `\\u0024\\u007b asd \\u0060\\u0060 \\u007d`</script>",
},
{
"JS template lit special characters, nested lit",
"<script>var a = `${ `{{.I}}` }`</script>",
"<script>var a = `${ `\\u0024\\u007b asd \\u0060\\u0060 \\u007d` }`</script>",
},
{
"JS template lit, nested JS",
"<script>var a = `${ var a = \"{{\"a \\\" d\"}}\" }`</script>",
"<script>var a = `${ var a = \"a \\u0022 d\" }`</script>",
},
}

for _, test := range tests {
Expand Down Expand Up @@ -976,6 +992,31 @@ func TestErrors(t *testing.T) {
"<script>var a = `${a+b}`</script>`",
"",
},
{
"<script>var tmpl = `asd`;</script>",
``,
},
{
"<script>var tmpl = `${1}`;</script>",
``,
},
{
"<script>var tmpl = `${return ``}`;</script>",
``,
},
{
"<script>var tmpl = `${return {{.}} }`;</script>",
``,
},
{
"<script>var tmpl = `${ let a = {1:1} {{.}} }`;</script>",
``,
},
{
"<script>var tmpl = `asd ${return \"{\"}`;</script>",
``,
},

// Error cases.
{
"{{if .Cond}}<a{{end}}",
Expand Down Expand Up @@ -1122,10 +1163,26 @@ func TestErrors(t *testing.T) {
// html is allowed since it is the last command in the pipeline, but urlquery is not.
`predefined escaper "urlquery" disallowed in template`,
},
{
"<script>var tmpl = `asd {{.}}`;</script>",
`{{.}} appears in a JS template literal`,
},
// {
// "<script>var tmpl = `asd {{.}}`;</script>",
// `{{.}} appears in a JS template literal`,
// },
// {
// "<script>var v = `${function(){return `{{.V}}+1`}()}`;</script>",
// `{{.V}} appears in a JS template literal`,
// },
// {
// "<script>var a = `asd ${function(){b = {1:2}; return`{{.}}`}}`</script>",
// `{{.}} appears in a JS template literal`,
// },
// {
// "<script>var tmpl = `${return `{{.}}`}`;</script>",
// `{{.}} appears in a JS template literal`,
// },
// {
// "<script>var tmpl = `${return {`{{.}}`}`;</script>",
// `{{.}} appears in a JS template literal`,
// },
}
for _, test := range tests {
buf := new(bytes.Buffer)
Expand Down Expand Up @@ -1349,7 +1406,7 @@ func TestEscapeText(t *testing.T) {
},
{
"<a onclick=\"`foo",
context{state: stateJSBqStr, delim: delimDoubleQuote, attr: attrScript},
context{state: stateJSTmplLit, delim: delimDoubleQuote, attr: attrScript},
},
{
`<A ONCLICK="'`,
Expand Down Expand Up @@ -1691,6 +1748,58 @@ func TestEscapeText(t *testing.T) {
`<svg:a svg:onclick="x()">`,
context{},
},
{
"<script>var a = `",
context{state: stateJSTmplLit, element: elementScript},
},
{
"<script>var a = `${",
context{state: stateJS, element: elementScript},
},
{
"<script>var a = `${}",
context{state: stateJSTmplLit, element: elementScript},
},
{
"<script>var a = `${`",
context{state: stateJSTmplLit, element: elementScript},
},
{
"<script>var a = `${var a = \"",
context{state: stateJSDqStr, element: elementScript},
},
{
"<script>var a = `${var a = \"`",
context{state: stateJSDqStr, element: elementScript},
},
{
"<script>var a = `${var a = \"}",
context{state: stateJSDqStr, element: elementScript},
},
{
"<script>var a = `${``",
context{state: stateJS, element: elementScript},
},
{
"<script>var a = `${`}",
context{state: stateJSTmplLit, element: elementScript},
},
{
"<script>`${ {} } asd`</script><script>`${ {} }",
context{state: stateJSTmplLit, element: elementScript},
},
{
"<script>var foo = `${ (_ => { return \"x\" })() + \"${",
context{state: stateJSDqStr, element: elementScript},
},
{
"<script>var a = `${ {</script><script>var b = `${ x }",
context{state: stateJSTmplLit, element: elementScript, jsCtx: jsCtxDivOp},
},
{
"<script>var foo = `x` + \"${",
context{state: stateJSDqStr, element: elementScript},
},
}

for _, test := range tests {
Expand Down
30 changes: 30 additions & 0 deletions src/html/template/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ func jsStrEscaper(args ...any) string {
return replace(s, jsStrReplacementTable)
}

func jsTmplLitEscaper(args ...any) string {
s, _ := stringify(args...)
return replace(s, jsBqStrReplacementTable)
}

// jsRegexpEscaper behaves like jsStrEscaper but escapes regular expression
// specials so the result is treated literally when included in a regular
// expression literal. /foo{{.X}}bar/ matches the string "foo" followed by
Expand Down Expand Up @@ -324,6 +329,31 @@ var jsStrReplacementTable = []string{
'\\': `\\`,
}

// jsBqStrReplacementTable is like jsStrReplacementTable except it also contains
// the special characters for JS template literals: $, {, and }.
var jsBqStrReplacementTable = []string{
0: `\u0000`,
'\t': `\t`,
'\n': `\n`,
'\v': `\u000b`, // "\v" == "v" on IE 6.
'\f': `\f`,
'\r': `\r`,
// Encode HTML specials as hex so the output can be embedded
// in HTML attributes without further encoding.
'"': `\u0022`,
'`': `\u0060`,
'&': `\u0026`,
'\'': `\u0027`,
'+': `\u002b`,
'/': `\/`,
'<': `\u003c`,
'>': `\u003e`,
'\\': `\\`,
'$': `\u0024`,
'{': `\u007b`,
'}': `\u007d`,
}

// jsStrNormReplacementTable is like jsStrReplacementTable but does not
// overencode existing escapes since this table has no entry for `\`.
var jsStrNormReplacementTable = []string{
Expand Down
6 changes: 3 additions & 3 deletions src/html/template/state_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit c9c885f

Please sign in to comment.