html/template: allow actions in JS template literals #61619

rolandshoemaker · 2023-07-27T19:12:19Z

This proposal is mutually exclusive with the already accepted proposal in #60055, but I believe is worth considering.

While prototyping the implementation for #60055 (see http://go.dev/cl/507995), we realized that it is actually not incredibly complicated to manage the state changes between template literals and nested string interpolation contexts (i.e. `${ ... }`) that would be necessary to allow us to safely support actions within JS template literals.

There are a handful of seemingly valid use cases for template literals which the current behavior (disallowing actions within template literals) breaks, namely it prevents using an action to construct a multiline string. With proper handling of template literals, we are in the position to possibly allow actions within them safely.

In order to prevent an action switching the context, strings inserted into the template literal context would need to have the characters $, {, } escaped. Actions in the string interpolation context would be treated the same as actions in the top-level JS context.

If accepted, the following templates would become acceptable (reverting the restrictions added in http://go.dev/cl/482079, and closing the existing loopholes).

// actions allowed within the string portion of the template literal,
// with the characters $, {, and } escaped.
<script>var a = `{{.}}`</script>

// actions allowed within the string interpolation portion of the
// template literal, with the same restrictions as the top-level JS context.
<script>var a = `${ {{.}} }`</script>

This would revert the restrictions added in http://go.dev/cl/482079, and deprecate ErrJSTemplate (which is, unfortunately, slated to be added in 1.21).

cc @golang/security @jupenur

The text was updated successfully, but these errors were encountered:

dwrz · 2023-07-29T12:34:12Z

I would prefer this proposal, if it's feasible. :)

I haven't tried them yet, but the three workarounds that have come up for multiline strings appear to be:

A script data block, as proposed by @jupenur: html/template: refuse to parse complex JS template literals #60055 (comment). I prefer this one, but it does require an element lookup, e.g., document.getElementById('date-row-template').innerHTML.
Installing a template function like the following:

package main

import "strings"

func multilineJS(input string) string {
	lines := strings.Split(input, "\n")

	// Escape single quotes.
	for i, line := range lines {
		lines[i] = strings.ReplaceAll(line, `'`, "\\'")
	}

	// Reassemble.
	return "'" + strings.Join(lines, "' + \n'") + "'"
}

func main() {
	input := `<tr>
<td>'你好世界'</td>
<td>{{ .Start }}</td>
<td>{{ .End }}</td>
</tr>`

	print(multilineJS(input))
}

But this gets trickier (1) if you want to handle single and double quotes, (2) if you want to preserve newlines in JavaScript, I think you need add them to the string:

`<tr>\n
<td>'你好世界'</td>\n`

For my use case, I've been trying to avoid using dynamic forms, but it's meant round-trips form submissions instead of using the client to add more input fields.

Regardless, I really appreciate the work and time on this issue. I use Go's templates because I much prefer them to JavaScript, trying to use the latter only when necessary.

rsc · 2023-08-09T21:42:00Z

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

rsc · 2023-08-09T21:42:23Z

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

rsc · 2023-08-16T17:24:24Z

As long as we can do this securely, this sounds like a win for everyone.

rsc · 2023-08-30T18:19:10Z

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

rsc · 2023-09-07T13:53:02Z

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

gopherbot · 2023-09-18T20:44:23Z

Change https://go.dev/cl/507995 mentions this issue: html/template: support parsing complex JS template literals

gopherbot · 2023-10-04T14:22:51Z

Change https://go.dev/cl/532595 mentions this issue: html/template: only track brace depth when we are in a JS tmpl lit

The change that keeps on giving. Only track brace depth in tJS if we are already inside of a template literal. If we start tracking depth outside of nested literals it can cause the parser to think we're still in a JS context when we've actually closed the string interp. I believe this _mostly_ captures the expected parsing, but since the JS parser does not implement proper lexical goal symbols, it may not be entirely accurate. At some point in the future we may be able to significantly reduce the complexity of this implementation by implementing a lexical parser that more closely follows the ECMAScript specification, and structuring escaping rules based on which symbol an action appears in. This would also allow us to catch errors, which we currently cannot reasonable do (although perhaps this is beyond the scope of what html/template _should_ be doing). Updates #61619 Change-Id: I56e1dbc0d0705ef8fb7a5454ebe2421d4e162ef6 Reviewed-on: https://go-review.googlesource.com/c/go/+/532595 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Damien Neil <dneil@google.com>

jupenur · 2023-10-05T09:13:41Z

@rolandshoemaker sorry for not getting to this sooner; I've been on PTO. The current (already merged) implementation is broken and escaping fails in this template:

<script>var foo = `${ foo({ a: { c: `${ {{.}} }` }, b: {{.}} })}`</script>

The new change at https://go.dev/cl/532595 doesn't seem to fix this either even though it does fiddle with the brace depth logic. The problem is that brace depth gets reset to 0 in places where there's nesting, and that breaks brace depth tracking for the outer JS template literals.

rolandshoemaker · 2023-10-05T12:34:44Z

Oh bah, you are correct, I thought I'd fixed this based on a particular case I was fixating on, but I realize now we actually need to track brace depth per-nesting level in order to properly address this.

gopherbot · 2023-10-05T13:26:18Z

Change https://go.dev/cl/532995 mentions this issue: html/template: track brace depth for each nested expression

rolandshoemaker · 2023-10-05T13:29:53Z

@jupenur I think http://go.dev/cl/532995 captures the correct behavior now, but each time I do this I'm less and less convinced I've gotten it right (which, given this is the... third(?) attempt at it, I've not got great odds). Enumerating all of the cases here is painful.

We need to track the brace depth for each individual nested expression, since a string interpolation expression may be nested inside of an object. e.g. `${ {1:`${}`}}` has brace depths [1, 0] when inside of the inner ${} expression. When we exit the inner expression, we need to reset to the previous brace depth (1) so that we know that the following } closes the object, but not the outer expression. Note that if you write a broken expression (i.e. `${ { }`) escaping will clearly not work as expected (or depending on your interpretation, since it is broken, it will work as expected). Since the JS parser doesn't catch syntax errors, it's up to the user to write a valid template. Updates #61619 Change-Id: I4c33723d12aff49facdcb1134d9ca82b7a0dffc4 Reviewed-on: https://go-review.googlesource.com/c/go/+/532995 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

We need to track the brace depth for each individual nested expression, since a string interpolation expression may be nested inside of an object. e.g. `${ {1:`${}`}}` has brace depths [1, 0] when inside of the inner ${} expression. When we exit the inner expression, we need to reset to the previous brace depth (1) so that we know that the following } closes the object, but not the outer expression. Note that if you write a broken expression (i.e. `${ { }`) escaping will clearly not work as expected (or depending on your interpretation, since it is broken, it will work as expected). Since the JS parser doesn't catch syntax errors, it's up to the user to write a valid template. Updates golang#61619 Change-Id: I4c33723d12aff49facdcb1134d9ca82b7a0dffc4 Reviewed-on: https://go-review.googlesource.com/c/go/+/532995 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

rolandshoemaker added the Proposal label Jul 27, 2023

rolandshoemaker added this to the Proposal milestone Jul 27, 2023

ianlancetaylor mentioned this issue Jul 27, 2023

html/template: refuse to parse complex JS template literals #60055

Open

rsc added the Proposal-FinalCommentPeriod label Aug 30, 2023

rsc changed the title ~~proposal: html/template: allow actions in JS template literals~~ html/template: allow actions in JS template literals Sep 7, 2023

rsc modified the milestones: Proposal, Backlog Sep 7, 2023

rsc added Proposal-Accepted and removed Proposal-FinalCommentPeriod labels Sep 7, 2023

gopherbot closed this as completed in c9c885f Oct 2, 2023

mknyszek mentioned this issue Nov 22, 2023

api: audit for Go 1.22 #64343

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

html/template: allow actions in JS template literals #61619

html/template: allow actions in JS template literals #61619

rolandshoemaker commented Jul 27, 2023

dwrz commented Jul 29, 2023

rsc commented Aug 9, 2023

rsc commented Aug 9, 2023

rsc commented Aug 16, 2023

rsc commented Aug 30, 2023

rsc commented Sep 7, 2023

gopherbot commented Sep 18, 2023

gopherbot commented Oct 4, 2023

jupenur commented Oct 5, 2023 •

edited

rolandshoemaker commented Oct 5, 2023

gopherbot commented Oct 5, 2023

rolandshoemaker commented Oct 5, 2023

html/template: allow actions in JS template literals #61619

html/template: allow actions in JS template literals #61619

Comments

rolandshoemaker commented Jul 27, 2023

dwrz commented Jul 29, 2023

rsc commented Aug 9, 2023

rsc commented Aug 9, 2023

rsc commented Aug 16, 2023

rsc commented Aug 30, 2023

rsc commented Sep 7, 2023

gopherbot commented Sep 18, 2023

gopherbot commented Oct 4, 2023

jupenur commented Oct 5, 2023 • edited

rolandshoemaker commented Oct 5, 2023

gopherbot commented Oct 5, 2023

rolandshoemaker commented Oct 5, 2023

jupenur commented Oct 5, 2023 •

edited