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

diff --git a/src/pkg/exp/template/html/context.go b/src/pkg/exp/template/html/context.go
index 856d1c94eb327131c5056784eac0009d70b15c14..19381d5d625629dc0238b56c92d831d90430f4a3 100644 (file)
--- a/src/pkg/exp/template/html/context.go
+++ b/src/pkg/exp/template/html/context.go
@@ -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)
 }

diff --git a/src/pkg/exp/template/html/escape.go b/src/pkg/exp/template/html/escape.go
index 955b41be22307a6dd38d360e28c46d38526ab000..c0a0a24dd2e1c97480a8d28658897529bfff0bbe 100644 (file)
--- a/src/pkg/exp/template/html/escape.go
+++ b/src/pkg/exp/template/html/escape.go
@@ -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")

diff --git a/src/pkg/exp/template/html/escape_test.go b/src/pkg/exp/template/html/escape_test.go
index 488f33a4add71fb59d13ceae861fcf1922d50429..5110b445cab2c388bf7f6d676c8be24684b4f49a 100644 (file)
--- a/src/pkg/exp/template/html/escape_test.go
+++ b/src/pkg/exp/template/html/escape_test.go
@@ -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 {