Commit 0432a23c authored by Mike Samuel's avatar Mike Samuel

exp/template/html: tolerate '/' ambiguity in JS when it doesn't matter.

Often, division/regexp ambiguity doesn't matter in JS because the next
token is not a slash.

For example, in

  <script>var global{{if .InitVal}} = {{.InitVal}}{{end}}</script>

When there is an initial value, the {{if}} ends with jsCtxDivOp
since a '/' following {{.InitVal}} would be a division operator.
When there is none, the empty {{else}} branch ends with jsCtxRegexp
since a '/' would start a regular expression.  A '/' could result
in a valid program if it were on a new line to allow semicolon
insertion to terminate the VarDeclaration.

There is no '/' though, so we can ignore the ambiguity.

There are cases where a missing semi can result in ambiguity that
we should report.

  <script>
  {{if .X}}var x = {{.X}}{{end}}
  /...{{.Y}}
  </script>

where ... could be /foo/.test(bar) or /divisor.  Disambiguating in
this case is hard and is required to sanitize {{.Y}}.

Note, that in the case where there is a '/' in the script tail but it
is not followed by any interpolation, we already don't care.  So we
are already tolerant of

<script>{{if .X}}var x = {{.X}}{{end}}/a-bunch-of-text</script>

because tJS checks for </script> before looking in /a-bunch-of-text.

This CL
- Adds a jsCtx value: jsCtxUnknown
- Changes joinContext to join contexts that only differ by jsCtx.
- Changes tJS to return an error when a '/' is seen in jsCtxUnknown.
- Adds tests for both the happy and sad cases.

R=nigeltao
CC=golang-dev
https://golang.org/cl/4956077
parent 80a5ddbd
......@@ -198,6 +198,8 @@ const (
jsCtxRegexp jsCtx = iota
// jsCtxDivOp occurs where a '/' would start a division operator.
jsCtxDivOp
// jsCtxUnknown occurs where a '/' is ambiguous due to context joining.
jsCtxUnknown
)
func (c jsCtx) String() string {
......@@ -206,6 +208,8 @@ func (c jsCtx) String() string {
return "jsCtxRegexp"
case jsCtxDivOp:
return "jsCtxDivOp"
case jsCtxUnknown:
return "jsCtxUnknown"
}
return fmt.Sprintf("illegal jsCtx %d", c)
}
......
......@@ -217,6 +217,14 @@ func join(a, b context, line int, nodeName string) context {
return c
}
c = a
c.jsCtx = b.jsCtx
if c.eq(b) {
// The contexts differ only by jsCtx.
c.jsCtx = jsCtxUnknown
return c
}
return context{
state: stateError,
errLine: line,
......@@ -492,8 +500,13 @@ func tJS(c context, s []byte) (context, []byte) {
c.state, i = stateJSBlockCmt, i+1
case c.jsCtx == jsCtxRegexp:
c.state = stateJSRegexp
default:
case c.jsCtx == jsCtxDivOp:
c.jsCtx = jsCtxRegexp
default:
return context{
state: stateError,
errStr: fmt.Sprintf("'/' could start div or regexp: %.32q", s[i:]),
}, nil
}
default:
panic("unreachable")
......
......@@ -202,6 +202,13 @@ func TestEscape(t *testing.T) {
`<script>alert(/{{""}}/.test(""));</script>`,
`<script>alert(/(?:)/.test(""));</script>`,
},
{
"jsReAmbigOk",
`<script>{{if true}}var x = 1{{end}}</script>`,
// The {if} ends in an ambiguous jsCtx but there is
// no slash following so we shouldn't care.
`<script>var x = 1</script>`,
},
{
"styleBidiKeywordPassed",
`<p style="dir: {{"ltr"}}">`,
......@@ -480,6 +487,15 @@ func TestErrors(t *testing.T) {
"<!-- {{.H}} -->",
"z:1: (action: [(command: [F=[H]])]) appears inside a comment",
},
{
// It is ambiguous whether 1.5 should be 1\.5 or 1.5.
// Either `var x = 1/- 1.5 /i.test(x)`
// where `i.test(x)` is a method call of reference i,
// or `/-1\.5/i.test(x)` which is a method call on a
// case insensitive regular expression.
`<script>{{if false}}var x = 1{{end}}/-{{"1.5"}}/i.test(x)</script>`,
`: '/' could start div or regexp: "/-"`,
},
}
for _, test := range tests {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment