Skip to content

Commit

Permalink
Fix upstream Go templates bug with reversed key/value assignment
Browse files Browse the repository at this point in the history
The template packages are based on go1.20.5 with the patch in 15d330e1aa7dea2a9ff981ec8130aeb06da900a1 added.

Fixes gohugoio#11112
  • Loading branch information
bep committed Jun 15, 2023
1 parent f73c567 commit 4f3d7ce
Show file tree
Hide file tree
Showing 20 changed files with 212 additions and 61 deletions.
2 changes: 2 additions & 0 deletions tpl/internal/go_templates/htmltemplate/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ const (
stateJSDqStr
// stateJSSqStr occurs inside a JavaScript single quoted string.
stateJSSqStr
// stateJSBqStr occurs inside a JavaScript back quoted string.
stateJSBqStr
// stateJSRegexp occurs inside a JavaScript regexp literal.
stateJSRegexp
// stateJSBlockCmt occurs inside a JavaScript /* block comment */.
Expand Down
2 changes: 1 addition & 1 deletion tpl/internal/go_templates/htmltemplate/css.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func cssValueFilter(args ...any) string {
// inside a string that might embed JavaScript source.
for i, c := range b {
switch c {
case 0, '"', '\'', '(', ')', '/', ';', '@', '[', '\\', ']', '`', '{', '}':
case 0, '"', '\'', '(', ')', '/', ';', '@', '[', '\\', ']', '`', '{', '}', '<', '>':
return filterFailsafe
case '-':
// Disallow <!-- or -->.
Expand Down
2 changes: 2 additions & 0 deletions tpl/internal/go_templates/htmltemplate/css_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,8 @@ func TestCSSValueFilter(t *testing.T) {
{`-exp\000052 ession(alert(1337))`, "ZgotmplZ"},
{`-expre\0000073sion`, "-expre\x073sion"},
{`@import url evil.css`, "ZgotmplZ"},
{"<", "ZgotmplZ"},
{">", "ZgotmplZ"},
}
for _, test := range tests {
got := cssValueFilter(test.css)
Expand Down
7 changes: 7 additions & 0 deletions tpl/internal/go_templates/htmltemplate/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,5 +231,12 @@ Least Surprise Property:
"A developer (or code reviewer) familiar with HTML, CSS, and JavaScript, who
knows that contextual autoescaping happens should be able to look at a {{.}}
and correctly infer what sanitization happens."
As a consequence of the Least Surprise Property, template actions within an
ECMAScript 6 template literal are disabled by default.
Handling string interpolation within these literals is rather complex resulting
in no clear safe way to support it.
To re-enable template actions within ECMAScript 6 template literals, use the
GODEBUG=jstmpllitinterp=1 environment variable.
*/
package template
13 changes: 13 additions & 0 deletions tpl/internal/go_templates/htmltemplate/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,19 @@ const (
// pipeline occurs in an unquoted attribute value context, "html" is
// disallowed. Avoid using "html" and "urlquery" entirely in new templates.
ErrPredefinedEscaper

// errJSTmplLit: "... appears in a JS template literal"
// Example:
// <script>var tmpl = `{{.Interp}`</script>
// Discussion:
// Package html/template does not support actions inside of JS template
// literals.
//
// TODO(rolandshoemaker): we cannot add this as an exported error in a minor
// release, since it is backwards incompatible with the other minor
// releases. As such we need to leave it unexported, and then we'll add it
// in the next major release.
errJSTmplLit
)

func (e *Error) Error() string {
Expand Down
16 changes: 13 additions & 3 deletions tpl/internal/go_templates/htmltemplate/escape.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ func (e *escaper) escape(c context, n parse.Node) context {
panic("escaping " + n.String() + " is unimplemented")
}

// var debugAllowActionJSTmpl = godebug.New("jstmpllitinterp")

// escapeAction escapes an action template node.
func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
if len(n.Pipe.Decl) != 0 {
Expand Down Expand Up @@ -224,6 +226,15 @@ 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" {
s = append(s, "_html_template_jsstrescaper")
} else {
return context{
state: stateError,
err: errorf(errJSTmplLit, n, n.Line, "%s appears in a JS template literal", n),
}
}
case stateJSRegexp:
s = append(s, "_html_template_jsregexpescaper")
case stateCSS:
Expand Down Expand Up @@ -370,9 +381,8 @@ func normalizeEscFn(e string) string {
// for all x.
var redundantFuncs = map[string]map[string]bool{
"_html_template_commentescaper": {
"_html_template_attrescaper": true,
"_html_template_nospaceescaper": true,
"_html_template_htmlescaper": true,
"_html_template_attrescaper": true,
"_html_template_htmlescaper": true,
},
"_html_template_cssescaper": {
"_html_template_attrescaper": true,
Expand Down
81 changes: 52 additions & 29 deletions tpl/internal/go_templates/htmltemplate/escape_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,38 +683,49 @@ func TestEscape(t *testing.T) {
`<img srcset={{",,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,"}}>`,
`<img srcset=,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,>`,
},
{
"unquoted empty attribute value (plaintext)",
"<p name={{.U}}>",
"<p name=ZgotmplZ>",
},
{
"unquoted empty attribute value (url)",
"<p href={{.U}}>",
"<p href=ZgotmplZ>",
},
{
"quoted empty attribute value",
"<p name=\"{{.U}}\">",
"<p name=\"\">",
},
}

for _, test := range tests {
tmpl := New(test.name)
tmpl = Must(tmpl.Parse(test.input))
// Check for bug 6459: Tree field was not set in Parse.
if tmpl.Tree != tmpl.text.Tree {
t.Errorf("%s: tree not set properly", test.name)
continue
}
b := new(strings.Builder)
if err := tmpl.Execute(b, data); err != nil {
t.Errorf("%s: template execution failed: %s", test.name, err)
continue
}
if w, g := test.output, b.String(); w != g {
t.Errorf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g)
continue
}
b.Reset()
if err := tmpl.Execute(b, pdata); err != nil {
t.Errorf("%s: template execution failed for pointer: %s", test.name, err)
continue
}
if w, g := test.output, b.String(); w != g {
t.Errorf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g)
continue
}
if tmpl.Tree != tmpl.text.Tree {
t.Errorf("%s: tree mismatch", test.name)
continue
}
t.Run(test.name, func(t *testing.T) {
tmpl := New(test.name)
tmpl = Must(tmpl.Parse(test.input))
// Check for bug 6459: Tree field was not set in Parse.
if tmpl.Tree != tmpl.text.Tree {
t.Fatalf("%s: tree not set properly", test.name)
}
b := new(strings.Builder)
if err := tmpl.Execute(b, data); err != nil {
t.Fatalf("%s: template execution failed: %s", test.name, err)
}
if w, g := test.output, b.String(); w != g {
t.Fatalf("%s: escaped output: want\n\t%q\ngot\n\t%q", test.name, w, g)
}
b.Reset()
if err := tmpl.Execute(b, pdata); err != nil {
t.Fatalf("%s: template execution failed for pointer: %s", test.name, err)
}
if w, g := test.output, b.String(); w != g {
t.Fatalf("%s: escaped output for pointer: want\n\t%q\ngot\n\t%q", test.name, w, g)
}
if tmpl.Tree != tmpl.text.Tree {
t.Fatalf("%s: tree mismatch", test.name)
}
})
}
}

Expand Down Expand Up @@ -941,6 +952,10 @@ func TestErrors(t *testing.T) {
"{{range .Items}}<a{{if .X}}{{end}}>{{if .X}}{{break}}{{end}}{{end}}",
"",
},
{
"<script>var a = `${a+b}`</script>`",
"",
},
// Error cases.
{
"{{if .Cond}}<a{{end}}",
Expand Down Expand Up @@ -1087,6 +1102,10 @@ 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`,
},
}
for _, test := range tests {
buf := new(bytes.Buffer)
Expand Down Expand Up @@ -1308,6 +1327,10 @@ func TestEscapeText(t *testing.T) {
`<a onclick="'foo&quot;`,
context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript},
},
{
"<a onclick=\"`foo",
context{state: stateJSBqStr, delim: delimDoubleQuote, attr: attrScript},
},
{
`<A ONCLICK="'`,
context{state: stateJSSqStr, delim: delimDoubleQuote, attr: attrScript},
Expand Down
3 changes: 3 additions & 0 deletions tpl/internal/go_templates/htmltemplate/html.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import (
// htmlNospaceEscaper escapes for inclusion in unquoted attribute values.
func htmlNospaceEscaper(args ...any) string {
s, t := stringify(args...)
if s == "" {
return filterFailsafe
}
if t == contentTypeHTML {
return htmlReplacer(stripTags(s), htmlNospaceNormReplacementTable, false)
}
Expand Down
4 changes: 4 additions & 0 deletions tpl/internal/go_templates/htmltemplate/hugo_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ import (
template "github.com/gohugoio/hugo/tpl/internal/go_templates/texttemplate"
)

// See https://github.com/golang/go/issues/59234
// Moved here to avoid dependency on Go's internal/debug package.
var debugAllowActionJSTmpl = false

/*
This files contains the Hugo related addons. All the other files in this
Expand Down
10 changes: 9 additions & 1 deletion tpl/internal/go_templates/htmltemplate/js.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import (
"unicode/utf8"
)

// jsWhitespace contains all of the JS whitespace characters, as defined
// by the \s character class.
// See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_expressions/Character_classes.
const jsWhitespace = "\f\n\r\t\v\u0020\u00a0\u1680\u2000\u2001\u2002\u2003\u2004\u2005\u2006\u2007\u2008\u2009\u200a\u2028\u2029\u202f\u205f\u3000\ufeff"

// nextJSCtx returns the context that determines whether a slash after the
// given run of tokens starts a regular expression instead of a division
// operator: / or /=.
Expand All @@ -27,7 +32,8 @@ import (
// JavaScript 2.0 lexical grammar and requires one token of lookbehind:
// https://www.mozilla.org/js/language/js20-2000-07/rationale/syntax.html
func nextJSCtx(s []byte, preceding jsCtx) jsCtx {
s = bytes.TrimRight(s, "\t\n\f\r \u2028\u2029")
// Trim all JS whitespace characters
s = bytes.TrimRight(s, jsWhitespace)
if len(s) == 0 {
return preceding
}
Expand Down Expand Up @@ -309,6 +315,7 @@ var jsStrReplacementTable = []string{
// Encode HTML specials as hex so the output can be embedded
// in HTML attributes without further encoding.
'"': `\u0022`,
'`': `\u0060`,
'&': `\u0026`,
'\'': `\u0027`,
'+': `\u002b`,
Expand All @@ -332,6 +339,7 @@ var jsStrNormReplacementTable = []string{
'"': `\u0022`,
'&': `\u0026`,
'\'': `\u0027`,
'`': `\u0060`,
'+': `\u002b`,
'/': `\/`,
'<': `\u003c`,
Expand Down
13 changes: 8 additions & 5 deletions tpl/internal/go_templates/htmltemplate/js_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,17 @@ func TestNextJsCtx(t *testing.T) {
{jsCtxDivOp, "0"},
// Dots that are part of a number are div preceders.
{jsCtxDivOp, "0."},
// Some JS interpreters treat NBSP as a normal space, so
// we must too in order to properly escape things.
{jsCtxRegexp, "=\u00A0"},
}

for _, test := range tests {
if nextJSCtx([]byte(test.s), jsCtxRegexp) != test.jsCtx {
t.Errorf("want %s got %q", test.jsCtx, test.s)
if ctx := nextJSCtx([]byte(test.s), jsCtxRegexp); ctx != test.jsCtx {
t.Errorf("%q: want %s got %s", test.s, test.jsCtx, ctx)
}
if nextJSCtx([]byte(test.s), jsCtxDivOp) != test.jsCtx {
t.Errorf("want %s got %q", test.jsCtx, test.s)
if ctx := nextJSCtx([]byte(test.s), jsCtxDivOp); ctx != test.jsCtx {
t.Errorf("%q: want %s got %s", test.s, test.jsCtx, ctx)
}
}

Expand Down Expand Up @@ -294,7 +297,7 @@ func TestEscapersOnLower7AndSelectHighCodepoints(t *testing.T) {
`0123456789:;\u003c=\u003e?` +
`@ABCDEFGHIJKLMNO` +
`PQRSTUVWXYZ[\\]^_` +
"`abcdefghijklmno" +
"\\u0060abcdefghijklmno" +
"pqrstuvwxyz{|}~\u007f" +
"\u00A0\u0100\\u2028\\u2029\ufeff\U0001D11E",
},
Expand Down
9 changes: 9 additions & 0 deletions tpl/internal/go_templates/htmltemplate/jsctx_string.go

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

37 changes: 35 additions & 2 deletions tpl/internal/go_templates/htmltemplate/state_string.go

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

7 changes: 6 additions & 1 deletion tpl/internal/go_templates/htmltemplate/transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var transitionFunc = [...]func(context, []byte) (context, int){
stateJS: tJS,
stateJSDqStr: tJSDelimited,
stateJSSqStr: tJSDelimited,
stateJSBqStr: tJSDelimited,
stateJSRegexp: tJSDelimited,
stateJSBlockCmt: tBlockCmt,
stateJSLineCmt: tLineCmt,
Expand Down Expand Up @@ -262,7 +263,7 @@ func tURL(c context, s []byte) (context, int) {

// tJS is the context transition function for the JS state.
func tJS(c context, s []byte) (context, int) {
i := bytes.IndexAny(s, `"'/`)
i := bytes.IndexAny(s, "\"`'/")
if i == -1 {
// Entire input is non string, comment, regexp tokens.
c.jsCtx = nextJSCtx(s, c.jsCtx)
Expand All @@ -274,6 +275,8 @@ func tJS(c context, s []byte) (context, int) {
c.state, c.jsCtx = stateJSDqStr, jsCtxRegexp
case '\'':
c.state, c.jsCtx = stateJSSqStr, jsCtxRegexp
case '`':
c.state, c.jsCtx = stateJSBqStr, jsCtxRegexp
case '/':
switch {
case i+1 < len(s) && s[i+1] == '/':
Expand Down Expand Up @@ -303,6 +306,8 @@ func tJSDelimited(c context, s []byte) (context, int) {
switch c.state {
case stateJSSqStr:
specials = `\'`
case stateJSBqStr:
specials = "`\\"
case stateJSRegexp:
specials = `\/[]`
}
Expand Down
Loading

0 comments on commit 4f3d7ce

Please sign in to comment.