Skip to content

Commit

Permalink
html/template,text/template: switch to Unicode escapes for JSON compa…
Browse files Browse the repository at this point in the history
…tibility

The existing implementation is not compatible with JSON
escape as it uses hex escaping.
Unicode escape, instead, is valid for both JSON and JS.
This fix avoids creating a separate escaping context for
scripts of type "application/ld+json" and it is more
future-proof in case more JSON+JS contexts get added
to the platform (e.g. import maps).

Fixes #33671
Fixes #37634

Change-Id: Id6f6524b4abc52e81d9d744d46bbe5bf2e081543
Reviewed-on: https://go-review.googlesource.com/c/go/+/226097
Reviewed-by: Carl Johnson <me@carlmjohnson.net>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
empijei authored and mvdan committed Apr 16, 2020
1 parent 71a6718 commit d4d2980
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 110 deletions.
70 changes: 35 additions & 35 deletions src/html/template/content_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestTypedContent(t *testing.T) {
HTML(`Hello, <b>World</b> &amp;tc!`),
HTMLAttr(` dir="ltr"`),
JS(`c && alert("Hello, World!");`),
JSStr(`Hello, World & O'Reilly\x21`),
JSStr(`Hello, World & O'Reilly\u0021`),
URL(`greeting=H%69,&addressee=(World)`),
Srcset(`greeting=H%69,&addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`),
URL(`,foo/,`),
Expand Down Expand Up @@ -70,7 +70,7 @@ func TestTypedContent(t *testing.T) {
`Hello, <b>World</b> &amp;tc!`,
` dir=&#34;ltr&#34;`,
`c &amp;&amp; alert(&#34;Hello, World!&#34;);`,
`Hello, World &amp; O&#39;Reilly\x21`,
`Hello, World &amp; O&#39;Reilly\u0021`,
`greeting=H%69,&amp;addressee=(World)`,
`greeting=H%69,&amp;addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`,
`,foo/,`,
Expand Down Expand Up @@ -100,7 +100,7 @@ func TestTypedContent(t *testing.T) {
`Hello,&#32;World&#32;&amp;tc!`,
`&#32;dir&#61;&#34;ltr&#34;`,
`c&#32;&amp;&amp;&#32;alert(&#34;Hello,&#32;World!&#34;);`,
`Hello,&#32;World&#32;&amp;&#32;O&#39;Reilly\x21`,
`Hello,&#32;World&#32;&amp;&#32;O&#39;Reilly\u0021`,
`greeting&#61;H%69,&amp;addressee&#61;(World)`,
`greeting&#61;H%69,&amp;addressee&#61;(World)&#32;2x,&#32;https://golang.org/favicon.ico&#32;500.5w`,
`,foo/,`,
Expand All @@ -115,7 +115,7 @@ func TestTypedContent(t *testing.T) {
`Hello, World &amp;tc!`,
` dir=&#34;ltr&#34;`,
`c &amp;&amp; alert(&#34;Hello, World!&#34;);`,
`Hello, World &amp; O&#39;Reilly\x21`,
`Hello, World &amp; O&#39;Reilly\u0021`,
`greeting=H%69,&amp;addressee=(World)`,
`greeting=H%69,&amp;addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`,
`,foo/,`,
Expand All @@ -130,7 +130,7 @@ func TestTypedContent(t *testing.T) {
`Hello, &lt;b&gt;World&lt;/b&gt; &amp;tc!`,
` dir=&#34;ltr&#34;`,
`c &amp;&amp; alert(&#34;Hello, World!&#34;);`,
`Hello, World &amp; O&#39;Reilly\x21`,
`Hello, World &amp; O&#39;Reilly\u0021`,
`greeting=H%69,&amp;addressee=(World)`,
`greeting=H%69,&amp;addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`,
`,foo/,`,
Expand All @@ -146,7 +146,7 @@ func TestTypedContent(t *testing.T) {
// Not escaped.
`c && alert("Hello, World!");`,
// Escape sequence not over-escaped.
`"Hello, World & O'Reilly\x21"`,
`"Hello, World & O'Reilly\u0021"`,
`"greeting=H%69,\u0026addressee=(World)"`,
`"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`,
`",foo/,"`,
Expand All @@ -162,7 +162,7 @@ func TestTypedContent(t *testing.T) {
// Not JS escaped but HTML escaped.
`c &amp;&amp; alert(&#34;Hello, World!&#34;);`,
// Escape sequence not over-escaped.
`&#34;Hello, World &amp; O&#39;Reilly\x21&#34;`,
`&#34;Hello, World &amp; O&#39;Reilly\u0021&#34;`,
`&#34;greeting=H%69,\u0026addressee=(World)&#34;`,
`&#34;greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w&#34;`,
`&#34;,foo/,&#34;`,
Expand All @@ -171,30 +171,30 @@ func TestTypedContent(t *testing.T) {
{
`<script>alert("{{.}}")</script>`,
[]string{
`\x3cb\x3e \x22foo%\x22 O\x27Reilly \x26bar;`,
`a[href =~ \x22\/\/example.com\x22]#foo`,
`Hello, \x3cb\x3eWorld\x3c\/b\x3e \x26amp;tc!`,
` dir=\x22ltr\x22`,
`c \x26\x26 alert(\x22Hello, World!\x22);`,
`\u003cb\u003e \u0022foo%\u0022 O\u0027Reilly \u0026bar;`,
`a[href =~ \u0022\/\/example.com\u0022]#foo`,
`Hello, \u003cb\u003eWorld\u003c\/b\u003e \u0026amp;tc!`,
` dir=\u0022ltr\u0022`,
`c \u0026\u0026 alert(\u0022Hello, World!\u0022);`,
// Escape sequence not over-escaped.
`Hello, World \x26 O\x27Reilly\x21`,
`greeting=H%69,\x26addressee=(World)`,
`greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`,
`Hello, World \u0026 O\u0027Reilly\u0021`,
`greeting=H%69,\u0026addressee=(World)`,
`greeting=H%69,\u0026addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`,
`,foo\/,`,
},
},
{
`<script type="text/javascript">alert("{{.}}")</script>`,
[]string{
`\x3cb\x3e \x22foo%\x22 O\x27Reilly \x26bar;`,
`a[href =~ \x22\/\/example.com\x22]#foo`,
`Hello, \x3cb\x3eWorld\x3c\/b\x3e \x26amp;tc!`,
` dir=\x22ltr\x22`,
`c \x26\x26 alert(\x22Hello, World!\x22);`,
`\u003cb\u003e \u0022foo%\u0022 O\u0027Reilly \u0026bar;`,
`a[href =~ \u0022\/\/example.com\u0022]#foo`,
`Hello, \u003cb\u003eWorld\u003c\/b\u003e \u0026amp;tc!`,
` dir=\u0022ltr\u0022`,
`c \u0026\u0026 alert(\u0022Hello, World!\u0022);`,
// Escape sequence not over-escaped.
`Hello, World \x26 O\x27Reilly\x21`,
`greeting=H%69,\x26addressee=(World)`,
`greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`,
`Hello, World \u0026 O\u0027Reilly\u0021`,
`greeting=H%69,\u0026addressee=(World)`,
`greeting=H%69,\u0026addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`,
`,foo\/,`,
},
},
Expand All @@ -208,7 +208,7 @@ func TestTypedContent(t *testing.T) {
// Not escaped.
`c && alert("Hello, World!");`,
// Escape sequence not over-escaped.
`"Hello, World & O'Reilly\x21"`,
`"Hello, World & O'Reilly\u0021"`,
`"greeting=H%69,\u0026addressee=(World)"`,
`"greeting=H%69,\u0026addressee=(World) 2x, https://golang.org/favicon.ico 500.5w"`,
`",foo/,"`,
Expand All @@ -224,7 +224,7 @@ func TestTypedContent(t *testing.T) {
`Hello, <b>World</b> &amp;tc!`,
` dir=&#34;ltr&#34;`,
`c &amp;&amp; alert(&#34;Hello, World!&#34;);`,
`Hello, World &amp; O&#39;Reilly\x21`,
`Hello, World &amp; O&#39;Reilly\u0021`,
`greeting=H%69,&amp;addressee=(World)`,
`greeting=H%69,&amp;addressee=(World) 2x, https://golang.org/favicon.ico 500.5w`,
`,foo/,`,
Expand All @@ -233,15 +233,15 @@ func TestTypedContent(t *testing.T) {
{
`<button onclick='alert("{{.}}")'>`,
[]string{
`\x3cb\x3e \x22foo%\x22 O\x27Reilly \x26bar;`,
`a[href =~ \x22\/\/example.com\x22]#foo`,
`Hello, \x3cb\x3eWorld\x3c\/b\x3e \x26amp;tc!`,
` dir=\x22ltr\x22`,
`c \x26\x26 alert(\x22Hello, World!\x22);`,
`\u003cb\u003e \u0022foo%\u0022 O\u0027Reilly \u0026bar;`,
`a[href =~ \u0022\/\/example.com\u0022]#foo`,
`Hello, \u003cb\u003eWorld\u003c\/b\u003e \u0026amp;tc!`,
` dir=\u0022ltr\u0022`,
`c \u0026\u0026 alert(\u0022Hello, World!\u0022);`,
// Escape sequence not over-escaped.
`Hello, World \x26 O\x27Reilly\x21`,
`greeting=H%69,\x26addressee=(World)`,
`greeting=H%69,\x26addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`,
`Hello, World \u0026 O\u0027Reilly\u0021`,
`greeting=H%69,\u0026addressee=(World)`,
`greeting=H%69,\u0026addressee=(World) 2x, https:\/\/golang.org\/favicon.ico 500.5w`,
`,foo\/,`,
},
},
Expand All @@ -253,7 +253,7 @@ func TestTypedContent(t *testing.T) {
`Hello%2c%20%3cb%3eWorld%3c%2fb%3e%20%26amp%3btc%21`,
`%20dir%3d%22ltr%22`,
`c%20%26%26%20alert%28%22Hello%2c%20World%21%22%29%3b`,
`Hello%2c%20World%20%26%20O%27Reilly%5cx21`,
`Hello%2c%20World%20%26%20O%27Reilly%5cu0021`,
// Quotes and parens are escaped but %69 is not over-escaped. HTML escaping is done.
`greeting=H%69,&amp;addressee=%28World%29`,
`greeting%3dH%2569%2c%26addressee%3d%28World%29%202x%2c%20https%3a%2f%2fgolang.org%2ffavicon.ico%20500.5w`,
Expand All @@ -268,7 +268,7 @@ func TestTypedContent(t *testing.T) {
`Hello%2c%20%3cb%3eWorld%3c%2fb%3e%20%26amp%3btc%21`,
`%20dir%3d%22ltr%22`,
`c%20%26%26%20alert%28%22Hello%2c%20World%21%22%29%3b`,
`Hello%2c%20World%20%26%20O%27Reilly%5cx21`,
`Hello%2c%20World%20%26%20O%27Reilly%5cu0021`,
// Quotes and parens are escaped but %69 is not over-escaped. HTML escaping is not done.
`greeting=H%69,&addressee=%28World%29`,
`greeting%3dH%2569%2c%26addressee%3d%28World%29%202x%2c%20https%3a%2f%2fgolang.org%2ffavicon.ico%20500.5w`,
Expand Down
6 changes: 3 additions & 3 deletions src/html/template/escape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func TestEscape(t *testing.T) {
{
"jsStr",
"<button onclick='alert(&quot;{{.H}}&quot;)'>",
`<button onclick='alert(&quot;\x3cHello\x3e&quot;)'>`,
`<button onclick='alert(&quot;\u003cHello\u003e&quot;)'>`,
},
{
"badMarshaler",
Expand All @@ -259,7 +259,7 @@ func TestEscape(t *testing.T) {
{
"jsRe",
`<button onclick='alert(/{{"foo+bar"}}/.test(""))'>`,
`<button onclick='alert(/foo\x2bbar/.test(""))'>`,
`<button onclick='alert(/foo\u002bbar/.test(""))'>`,
},
{
"jsReBlank",
Expand Down Expand Up @@ -825,7 +825,7 @@ func TestEscapeSet(t *testing.T) {
"main": `<button onclick="title='{{template "helper"}}'; ...">{{template "helper"}}</button>`,
"helper": `{{11}} of {{"<100>"}}`,
},
`<button onclick="title='11 of \x3c100\x3e'; ...">11 of &lt;100&gt;</button>`,
`<button onclick="title='11 of \u003c100\u003e'; ...">11 of &lt;100&gt;</button>`,
},
// A non-recursive template that ends in a different context.
// helper starts in jsCtxRegexp and ends in jsCtxDivOp.
Expand Down
6 changes: 3 additions & 3 deletions src/html/template/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ func Example_escape() {
// &#34;Fran &amp; Freddie&#39;s Diner&#34; &lt;tasty@example.com&gt;
// &#34;Fran &amp; Freddie&#39;s Diner&#34; &lt;tasty@example.com&gt;
// &#34;Fran &amp; Freddie&#39;s Diner&#34;32&lt;tasty@example.com&gt;
// \"Fran \x26 Freddie\'s Diner\" \x3Ctasty@example.com\x3E
// \"Fran \x26 Freddie\'s Diner\" \x3Ctasty@example.com\x3E
// \"Fran \x26 Freddie\'s Diner\"32\x3Ctasty@example.com\x3E
// \"Fran \u0026 Freddie\'s Diner\" \u003Ctasty@example.com\u003E
// \"Fran \u0026 Freddie\'s Diner\" \u003Ctasty@example.com\u003E
// \"Fran \u0026 Freddie\'s Diner\"32\u003Ctasty@example.com\u003E
// %22Fran+%26+Freddie%27s+Diner%2232%3Ctasty%40example.com%3E

}
Expand Down
70 changes: 42 additions & 28 deletions src/html/template/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ func jsValEscaper(args ...interface{}) string {
}
// TODO: detect cycles before calling Marshal which loops infinitely on
// cyclic data. This may be an unacceptable DoS risk.

b, err := json.Marshal(a)
if err != nil {
// Put a space before comment so that if it is flush against
Expand All @@ -178,8 +177,8 @@ func jsValEscaper(args ...interface{}) string {
// TODO: maybe post-process output to prevent it from containing
// "<!--", "-->", "<![CDATA[", "]]>", or "</script"
// in case custom marshalers produce output containing those.

// TODO: Maybe abbreviate \u00ab to \xab to produce more compact output.
// Note: Do not use \x escaping to save bytes because it is not JSON compatible and this escaper
// supports ld+json content-type.
if len(b) == 0 {
// In, `x=y/{{.}}*z` a json.Marshaler that produces "" should
// not cause the output `x=y/*z`.
Expand Down Expand Up @@ -260,6 +259,8 @@ func replace(s string, replacementTable []string) string {
r, w = utf8.DecodeRuneInString(s[i:])
var repl string
switch {
case int(r) < len(lowUnicodeReplacementTable):
repl = lowUnicodeReplacementTable[r]
case int(r) < len(replacementTable) && replacementTable[r] != "":
repl = replacementTable[r]
case r == '\u2028':
Expand All @@ -283,67 +284,80 @@ func replace(s string, replacementTable []string) string {
return b.String()
}

var lowUnicodeReplacementTable = []string{
0: `\u0000`, 1: `\u0001`, 2: `\u0002`, 3: `\u0003`, 4: `\u0004`, 5: `\u0005`, 6: `\u0006`,
'\a': `\u0007`,
'\b': `\u0008`,
'\t': `\t`,
'\n': `\n`,
'\v': `\u000b`, // "\v" == "v" on IE 6.
'\f': `\f`,
'\r': `\r`,
0xe: `\u000e`, 0xf: `\u000f`, 0x10: `\u0010`, 0x11: `\u0011`, 0x12: `\u0012`, 0x13: `\u0013`,
0x14: `\u0014`, 0x15: `\u0015`, 0x16: `\u0016`, 0x17: `\u0017`, 0x18: `\u0018`, 0x19: `\u0019`,
0x1a: `\u001a`, 0x1b: `\u001b`, 0x1c: `\u001c`, 0x1d: `\u001d`, 0x1e: `\u001e`, 0x1f: `\u001f`,
}

var jsStrReplacementTable = []string{
0: `\0`,
0: `\u0000`,
'\t': `\t`,
'\n': `\n`,
'\v': `\x0b`, // "\v" == "v" on IE 6.
'\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.
'"': `\x22`,
'&': `\x26`,
'\'': `\x27`,
'+': `\x2b`,
'"': `\u0022`,
'&': `\u0026`,
'\'': `\u0027`,
'+': `\u002b`,
'/': `\/`,
'<': `\x3c`,
'>': `\x3e`,
'<': `\u003c`,
'>': `\u003e`,
'\\': `\\`,
}

// jsStrNormReplacementTable is like jsStrReplacementTable but does not
// overencode existing escapes since this table has no entry for `\`.
var jsStrNormReplacementTable = []string{
0: `\0`,
0: `\u0000`,
'\t': `\t`,
'\n': `\n`,
'\v': `\x0b`, // "\v" == "v" on IE 6.
'\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.
'"': `\x22`,
'&': `\x26`,
'\'': `\x27`,
'+': `\x2b`,
'"': `\u0022`,
'&': `\u0026`,
'\'': `\u0027`,
'+': `\u002b`,
'/': `\/`,
'<': `\x3c`,
'>': `\x3e`,
'<': `\u003c`,
'>': `\u003e`,
}

var jsRegexpReplacementTable = []string{
0: `\0`,
0: `\u0000`,
'\t': `\t`,
'\n': `\n`,
'\v': `\x0b`, // "\v" == "v" on IE 6.
'\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.
'"': `\x22`,
'"': `\u0022`,
'$': `\$`,
'&': `\x26`,
'\'': `\x27`,
'&': `\u0026`,
'\'': `\u0027`,
'(': `\(`,
')': `\)`,
'*': `\*`,
'+': `\x2b`,
'+': `\u002b`,
'-': `\-`,
'.': `\.`,
'/': `\/`,
'<': `\x3c`,
'>': `\x3e`,
'<': `\u003c`,
'>': `\u003e`,
'?': `\?`,
'[': `\[`,
'\\': `\\`,
Expand Down
Loading

0 comments on commit d4d2980

Please sign in to comment.