diff --git a/src/pkg/exp/template/html/escape.go b/src/pkg/exp/template/html/escape.go index a0fccf96d1..52d6323fae 100644 --- a/src/pkg/exp/template/html/escape.go +++ b/src/pkg/exp/template/html/escape.go @@ -23,6 +23,9 @@ func Escape(t *template.Template) (*template.Template, os.Error) { if c.errStr != "" { return nil, fmt.Errorf("%s:%d: %s", t.Name(), c.errLine, c.errStr) } + if c.state != stateText { + return nil, fmt.Errorf("%s ends in a non-text context: %v", t.Name(), c) + } return t, nil } @@ -38,7 +41,7 @@ func escape(c context, n parse.Node) context { case *parse.RangeNode: return escapeBranch(c, &n.BranchNode, "range") case *parse.TextNode: - return escapeText(c, n) + return escapeText(c, n.Text) case *parse.WithNode: return escapeBranch(c, &n.BranchNode, "with") } @@ -91,11 +94,19 @@ func join(a, b context, line int, nodeName string) context { // escapeBranch escapes a branch template node: "if", "range" and "with". func escapeBranch(c context, n *parse.BranchNode, nodeName string) context { c0 := escapeList(c, n.List) - if nodeName == "range" { + if nodeName == "range" && c0.state != stateError { // The "true" branch of a "range" node can execute multiple times. // We check that executing n.List once results in the same context // as executing n.List twice. c0 = join(c0, escapeList(c0, n.List), n.Line, nodeName) + if c0.state == stateError { + // Make clear that this is a problem on loop re-entry + // since developers tend to overlook that branch when + // debugging templates. + c0.errLine = n.Line + c0.errStr = "on range loop re-entry: " + c0.errStr + return c0 + } } c1 := escapeList(c, n.ElseList) return join(c0, c1, n.Line, nodeName) @@ -112,10 +123,40 @@ func escapeList(c context, n *parse.ListNode) context { return c } +// delimEnds maps each delim to a string of characters that terminate it. +var delimEnds = [...]string{ + delimDoubleQuote: `"`, + delimSingleQuote: "'", + // Determined empirically by running the below in various browsers. + // var div = document.createElement("DIV"); + // for (var i = 0; i < 0x10000; ++i) { + // div.innerHTML = ""; + // if (div.getElementsByTagName("SPAN")[0].title.indexOf("bar") < 0) + // document.write("

U+" + i.toString(16)); + // } + delimSpaceOrTagEnd: " \t\n\f\r>", +} + // escapeText escapes a text template node. -func escapeText(c context, n *parse.TextNode) context { - for s := n.Text; len(s) > 0; { - c, s = transitionFunc[c.state](c, s) +func escapeText(c context, s []byte) context { + for len(s) > 0 { + if c.delim == delimNone { + c, s = transitionFunc[c.state](c, s) + continue + } + + i := bytes.IndexAny(s, delimEnds[c.delim]) + if i == -1 { + // Remain inside the attribute. + // TODO: Recurse to take into account grammars for + // JS, CSS, URIs embedded in attrs once implemented. + return c + } + if c.delim != delimSpaceOrTagEnd { + // Consume any quote. + i++ + } + c, s = context{state: stateTag}, s[i:] } return c } @@ -157,15 +198,15 @@ func tText(c context, s []byte) (context, []byte) { // tTag is the context transition function for the tag state. func tTag(c context, s []byte) (context, []byte) { - // Skip to the end tag, if there is one. - i := bytes.IndexByte(s, '>') - if i != -1 { - return context{state: stateText}, s[i+1:] + // Find the attribute name. + attrStart := eatWhiteSpace(s, 0) + i, err := eatAttrName(s, attrStart) + if err != nil { + return context{ + state: stateError, + errStr: err.String(), + }, nil } - - // Otherwise, find the attribute name. - i = eatWhiteSpace(s, 0) - attrStart, i := i, eatAttrName(s, i) if i == len(s) { return context{state: stateTag}, nil } @@ -174,37 +215,44 @@ func tTag(c context, s []byte) (context, []byte) { state = stateURL } - // Consume the "=". + // Look for the start of the value. i = eatWhiteSpace(s, i) - if i == len(s) || s[i] != '=' { + if i == len(s) { return context{state: stateTag}, s[i:] } + if s[i] == '>' { + return context{state: stateText}, s[i+1:] + } else if s[i] != '=' { + // Possible due to a valueless attribute or '/' in "". + return context{state: stateTag}, s[i:] + } + // Consume the "=". i = eatWhiteSpace(s, i+1) - // Find the delimiter. - if i == len(s) { - return context{state: state, delim: delimSpaceOrTagEnd}, nil - } - switch s[i] { - case '\'': - return context{state: state, delim: delimSingleQuote}, s[i+1:] - case '"': - return context{state: state, delim: delimDoubleQuote}, s[i+1:] + // Find the attribute delimiter. + if i < len(s) { + switch s[i] { + case '\'': + return context{state: state, delim: delimSingleQuote}, s[i+1:] + case '"': + return context{state: state, delim: delimDoubleQuote}, s[i+1:] + } } - // TODO: This shouldn't be an error: `': + return j, nil + case '\'', '"', '<': + // These result in a parse warning in HTML5 and are + // indicative of serious problems if seen in an attr + // name in a template. + return 0, fmt.Errorf("%q in attribute name: %.32q", s[j:j+1], s) default: // No-op. } } - return len(s) + return len(s), nil } // eatTagName returns the largest j such that s[i:j] is a tag name. @@ -248,7 +304,7 @@ func eatTagName(s []byte, i int) int { func eatWhiteSpace(s []byte, i int) int { for j := i; j < len(s); j++ { switch s[j] { - case ' ', '\n', '\r', '\t': + case ' ', '\t', '\n', '\f', '\r': // No-op. default: return j diff --git a/src/pkg/exp/template/html/escape_test.go b/src/pkg/exp/template/html/escape_test.go index e3b68d6318..ee36da2257 100644 --- a/src/pkg/exp/template/html/escape_test.go +++ b/src/pkg/exp/template/html/escape_test.go @@ -8,7 +8,6 @@ import ( "bytes" "strings" "template" - "template/parse" "testing" ) @@ -87,6 +86,11 @@ func TestEscape(t *testing.T) { ``, ``, }, + { + "multipleAttrs", + "", + "", + }, } for _, tc := range testCases { @@ -147,15 +151,11 @@ func TestErrors(t *testing.T) { "{{if .Cond}}\n{{else}}\n{{else}}{{else}}", @@ -171,11 +171,15 @@ func TestErrors(t *testing.T) { }, { "{{range .Items}}