exp/template/html: moved error docs out of package docs onto error codes
This replaces the errStr & errLine members of context with a single err
*Error, and introduces a number of const error codes, one per
escape-time failure mode, that can be separately documented.
The changes to the error documentation moved from doc.go to error.go
are cosmetic.
R=r, nigeltao
CC=golang-dev
https://golang.org/cl/5026041
diff --git a/src/pkg/exp/template/html/Makefile b/src/pkg/exp/template/html/Makefile
index e53270c..9032aea 100644
--- a/src/pkg/exp/template/html/Makefile
+++ b/src/pkg/exp/template/html/Makefile
@@ -11,6 +11,7 @@
context.go\
css.go\
doc.go\
+ error.go\
escape.go\
html.go\
js.go\
diff --git a/src/pkg/exp/template/html/context.go b/src/pkg/exp/template/html/context.go
index bfe168f..e8812cf 100644
--- a/src/pkg/exp/template/html/context.go
+++ b/src/pkg/exp/template/html/context.go
@@ -21,8 +21,7 @@
urlPart urlPart
jsCtx jsCtx
element element
- errLine int
- errStr string
+ err *Error
}
// eq returns whether two contexts are equal.
@@ -32,8 +31,7 @@
c.urlPart == d.urlPart &&
c.jsCtx == d.jsCtx &&
c.element == d.element &&
- c.errLine == d.errLine &&
- c.errStr == d.errStr
+ c.err == d.err
}
// mangle produces an identifier that includes a suffix that distinguishes it
diff --git a/src/pkg/exp/template/html/doc.go b/src/pkg/exp/template/html/doc.go
index 2751ce8..a9b78ca 100644
--- a/src/pkg/exp/template/html/doc.go
+++ b/src/pkg/exp/template/html/doc.go
@@ -69,178 +69,8 @@
Errors
-This section describes the errors returned by EscapeSet. Each error is
-illustrated by an example that triggers the error, followed by an explanation
-of the problem.
+See the documentation of ErrorCode for details.
-Error: "... appears in an ambiguous URL context"
-Example:
- <a href="
- {{if .C}}
- /path/
- {{else}}
- /search?q=
- {{end}}
- {{.X}}
- ">
-Discussion:
- {{.X}} is in an ambiguous URL context since, depending on {{.C}}, it may be
- either a URL suffix or a query parameter.
- Moving {{.X}} into the condition removes the ambiguity:
- <a href="{{if .C}}/path/{{.X}}{{else}}/search?q={{.X}}">
-
-
-Error: "... appears inside a comment"
-Example:
-*/
-// <!-- {{.X}} -->
-// <script>/* {{.X}} */</script>
-// <style>/* {{.X}} */</style>
-/*
-Discussion:
- {{.X}} appears inside a comment. There is no escaping convention for
- comments. To use IE conditional comments, inject the
- whole comment as a type string (see below).
- To comment out code, break the {{...}}.
-
-Error: "{{if}} branches end in different contexts"
-Example:
- {{if .C}}<a href="{{end}}{{.X}}
-Discussion:
- EscapeSet statically examines each possible path when it encounters a {{if}},
- {{range}}, or {{with}} to escape any following pipelines. The example is
- ambiguous since {{.X}} might be an HTML text node, or a URL prefix in an
- HTML attribute. EscapeSet needs to understand the context of {{.X}} to escape
- it, but that depends on the run-time value of {{.C}}.
-
- The problem is usually something like missing quotes or angle brackets, or
- can be avoided by refactoring to put the two contexts into different
- branches of an if, range or with. Adding an {{else}} might help.
-
- First, look for a bug in your template. Missing quotes or '>' can trigger
- this error.
-
- {{if .C}}<div ... class="foo>{{end}} <- No quote after foo
-
- Second, try refactoring your template.
-
- {{if .C}}<script>alert({{end}}{{.X}}{{if .C}})</script>{{end}}
-
- ->
-
- {{if .C}}<script>alert({{.X}})</script>{{else}}{{.X}}{{end}}
-
- Third, check for {{range}}s that have no {{else}}
-
- <a href="/search
- {{range $i, $v := .}}
- {{if $i}}&{{else}}?{{end}}
- v={{$v}}
- {{end}}
- &x={{.X}}
- ">
-
- looks good, but if {{.}} is empty then the URL is /search&x=...
- where {{.X}} is not guaranteed to be in a URL query.
- EscapeSet cannot prove which {{range}} collections are never non-empty, so
- add an {{else}}
-
- <a href="{{range ...}}...{{end}}&x={{X}}">
-
- ->
-
- <a href="{{range ...}}...{{else}}?{{end}}&x={{.X}}">
-
- Fourth, contact the mailing list. You may have a useful pattern that
- EscapeSet does not yet support, and we can work with you.
-
-
-Error: "... ends in a non-text context: ..."
-Examples:
- <div
- <div title="no close quote>
- <script>f()
-Discussion:
- EscapeSet assumes the ouput is a DocumentFragment of HTML.
- Templates that end without closing tags will trigger this warning.
- Templates that produce incomplete Fragments should not be named
- in the call to EscapeSet.
-
-
-If you have a helper template in your set that is not meant to produce a
- document fragment, then do not pass its name to EscapeSet(set, ...names).
-
- {{define "main"}} <script>{{template "helper"}}</script> {{end}}
- {{define "helper"}} document.write(' <div title=" ') {{end}}
-
- "helper" does not produce a valid document fragment, though it does
- produce a valid JavaScript Program.
-
-"must specify names of top level templates"
-
- EscapeSet does not assume that all templates in a set produce HTML.
- Some may be helpers that produce snippets of other languages.
- Passing in no template names is most likely an error, so EscapeSet(set) will
- panic.
- If you call EscapeSet with a slice of names, guard it with a len check:
-
- if len(names) != 0 {
- set, err := EscapeSet(set, ...names)
- }
-
-Error: "no such template ..."
-Examples:
- {{define "main"}}<div {{template "attrs"}}>{{end}}
- {{define "attrs"}}href="{{.URL}}"{{end}}
-Discussion:
- EscapeSet looks through template calls to compute the context.
- Here the {{.URL}} in "attrs" must be treated as a URL when called from "main",
- but if "attrs" is not in set when EscapeSet(&set, "main") is called, this
- error will arise.
-
-Error: "on range loop re-entry: ..."
-Example:
- {{range .}}<p class={{.}}{{end}}
-Discussion:
- If an iteration through a range would cause it to end in
- a different context than an earlier pass, there is no single context.
- In the example, the <p> tag is missing a '>'.
- EscapeSet cannot tell whether {{.}} is meant to be an HTML class or the
- content of a broken <p> element and complains because the second iteration
- would produce something like
-
- <p class=foo<p class=bar
-
-Error: "unfinished escape sequence in ..."
-Example:
- <script>alert("\{{.X}}")</script>
-Discussion:
- EscapeSet does not support actions following a backslash.
- This is usually an error and there are better solutions; for
- our example
- <script>alert("{{.X}}")</script>
- should work, and if {{.X}} is a partial escape sequence such as
- "xA0", give it the type ContentTypeJSStr and include the whole
- sequence, as in
- {`\xA0`, ContentTypeJSStr}
-
-Error: "unfinished JS regexp charset in ..."
-Example:
- <script>var pattern = /foo[{{.Chars}}]/</script>
-Discussion:
- EscapeSet does not support interpolation into regular expression literal
- character sets.
-
-Error: "ZgotmplZ"
-Example:
- <img src="{{.X}}">
- where {{.X}} evaluates to `javascript:...`
-Discussion:
- "ZgotmplZ" is a special value that indicates that unsafe content reached
- a CSS or URL context at runtime. The output of the example will be
- <img src="#ZgotmplZ">
- If the data can be trusted, giving the string type XXX will exempt
- it from filtering.
A fuller picture
@@ -249,8 +79,6 @@
will not need to understand these details.
-
-
Contexts
Assuming {{.}} is `O'Reilly: How are <i>you</i>?`, the table below shows
diff --git a/src/pkg/exp/template/html/error.go b/src/pkg/exp/template/html/error.go
new file mode 100644
index 0000000..5fa2357
--- /dev/null
+++ b/src/pkg/exp/template/html/error.go
@@ -0,0 +1,194 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package html
+
+import (
+ "fmt"
+)
+
+// Error describes a problem encountered during template Escaping.
+type Error struct {
+ // ErrorCode describes the kind of error.
+ ErrorCode ErrorCode
+ // Name is the name of the template in which the error was encountered.
+ Name string
+ // Line is the line number of the error in the template source or 0.
+ Line int
+ // Description is a human-readable description of the problem.
+ Description string
+}
+
+// ErrorCode is a code for a kind of error.
+type ErrorCode int
+
+// We define codes for each error that manifests while escaping templates, but
+// escaped templates may also fail at runtime.
+//
+// Output: "ZgotmplZ"
+// Example:
+// <img src="{{.X}}">
+// where {{.X}} evaluates to `javascript:...`
+// Discussion:
+// "ZgotmplZ" is a special value that indicates that unsafe content reached a
+// CSS or URL context at runtime. The output of the example will be
+// <img src="#ZgotmplZ">
+// If the data comes from a trusted source, use content types to exempt it
+// from filtering: URL(`javascript:...`).
+const (
+ // OK indicates the lack of an error.
+ OK ErrorCode = iota
+
+ // ErrorAmbigContext: "... appears in an ambiguous URL context"
+ // Example:
+ // <a href="
+ // {{if .C}}
+ // /path/
+ // {{else}}
+ // /search?q=
+ // {{end}}
+ // {{.X}}
+ // ">
+ // Discussion:
+ // {{.X}} is in an ambiguous URL context since, depending on {{.C}},
+ // it may be either a URL suffix or a query parameter.
+ // Moving {{.X}} into the condition removes the ambiguity:
+ // <a href="{{if .C}}/path/{{.X}}{{else}}/search?q={{.X}}">
+ ErrAmbigContext
+
+ // TODO: document
+ ErrBadHTML
+
+ // ErrBranchEnd: "{{if}} branches end in different contexts"
+ // Example:
+ // {{if .C}}<a href="{{end}}{{.X}}
+ // Discussion:
+ // EscapeSet statically examines each possible path when it encounters
+ // a {{if}}, {{range}}, or {{with}} to escape any following pipelines.
+ // The example is ambiguous since {{.X}} might be an HTML text node,
+ // or a URL prefix in an HTML attribute. EscapeSet needs to understand
+ // the context of {{.X}} to escape it, but that depends on the
+ // run-time value of {{.C}}.
+ //
+ // The problem is usually something like missing quotes or angle
+ // brackets, or can be avoided by refactoring to put the two contexts
+ // into different branches of an if, range or with. If the problem
+ // is in a {{range}} over a collection that should never be empty,
+ // adding a dummy {{else}} can help.
+ ErrBranchEnd
+
+ // ErrEndContext: "... ends in a non-text context: ..."
+ // Examples:
+ // <div
+ // <div title="no close quote>
+ // <script>f()
+ // Discussion:
+ // EscapeSet assumes the ouput is a DocumentFragment of HTML.
+ // Templates that end without closing tags will trigger this error.
+ // Templates that produce incomplete Fragments should not be named
+ // in the call to EscapeSet.
+ //
+ // If you have a helper template in your set that is not meant to
+ // produce a document fragment, then do not pass its name to
+ // EscapeSet(set, ...names).
+ //
+ // {{define "main"}} <script>{{template "helper"}}</script> {{end}}
+ // {{define "helper"}} document.write(' <div title=" ') {{end}}
+ //
+ // "helper" does not produce a valid document fragment, though it does
+ // produce a valid JavaScript Program.
+ ErrEndContext
+
+ // ErrInsideComment: "... appears inside a comment"
+ // Example:
+ // <!-- {{.X}} -->
+ // <script>/* {{.X}} */</script>
+ // <style>/* {{.X}} */</style>
+ //
+ // Discussion:
+ // {{.X}} appears inside a comment. There is no escaping convention for
+ // comments. To use IE conditional comments, inject the whole comment
+ // as an HTML, JS, or CSS value (see content.go).
+ // To comment out code, break the {{...}}.
+ ErrInsideComment
+
+ // ErrNoNames: "must specify names of top level templates"
+ //
+ // EscapeSet does not assume that all templates in a set produce HTML.
+ // Some may be helpers that produce snippets of other languages.
+ // Passing in no template names is most likely an error,
+ // so EscapeSet(set) will panic.
+ // If you call EscapeSet with a slice of names, guard it with len:
+ //
+ // if len(names) != 0 {
+ // set, err := EscapeSet(set, ...names)
+ // }
+ ErrNoNames
+
+ // ErrNoSuchTemplate: "no such template ..."
+ // Examples:
+ // {{define "main"}}<div {{template "attrs"}}>{{end}}
+ // {{define "attrs"}}href="{{.URL}}"{{end}}
+ // Discussion:
+ // EscapeSet looks through template calls to compute the context.
+ // Here the {{.URL}} in "attrs" must be treated as a URL when called
+ // from "main", but if "attrs" is not in set when
+ // EscapeSet(&set, "main") is called, this error will arise.
+ ErrNoSuchTemplate
+
+ // TODO: document
+ ErrOutputContext
+
+ // ErrPartialCharset: "unfinished JS regexp charset in ..."
+ // Example:
+ // <script>var pattern = /foo[{{.Chars}}]/</script>
+ // Discussion:
+ // EscapeSet does not support interpolation into regular expression
+ // literal character sets.
+ ErrPartialCharset
+
+ // ErrPartialEscape: "unfinished escape sequence in ..."
+ // Example:
+ // <script>alert("\{{.X}}")</script>
+ // Discussion:
+ // EscapeSet does not support actions following a backslash.
+ // This is usually an error and there are better solutions; for
+ // our example
+ // <script>alert("{{.X}}")</script>
+ // should work, and if {{.X}} is a partial escape sequence such as
+ // "xA0", mark the whole sequence as safe content: JSStr(`\xA0`)
+ ErrPartialEscape
+
+ // ErrRangeLoopReentry: "on range loop re-entry: ..."
+ // Example:
+ // {{range .}}<p class={{.}}{{end}}
+ // Discussion:
+ // If an iteration through a range would cause it to end in a
+ // different context than an earlier pass, there is no single context.
+ // In the example, the <p> tag is missing a '>'.
+ // EscapeSet cannot tell whether {{.}} is meant to be an HTML class or
+ // the content of a broken <p> element and complains because the
+ // second iteration would produce something like
+ //
+ // <p class=foo<p class=bar
+ ErrRangeLoopReentry
+
+ // TODO: document
+ ErrSlashAmbig
+)
+
+func (e *Error) String() string {
+ if e.Line != 0 {
+ return fmt.Sprintf("exp/template/html:%s:%d: %s", e.Name, e.Line, e.Description)
+ } else if e.Name != "" {
+ return fmt.Sprintf("exp/template/html:%s: %s", e.Name, e.Description)
+ }
+ return "exp/template/html: " + e.Description
+}
+
+// errorf creates an error given a format string f and args.
+// The template Name still needs to be supplied.
+func errorf(k ErrorCode, line int, f string, args ...interface{}) *Error {
+ return &Error{k, "", line, fmt.Sprintf(f, args...)}
+}
diff --git a/src/pkg/exp/template/html/escape.go b/src/pkg/exp/template/html/escape.go
index b0acf48..3fa92cc 100644
--- a/src/pkg/exp/template/html/escape.go
+++ b/src/pkg/exp/template/html/escape.go
@@ -36,7 +36,7 @@
if len(names) == 0 {
// TODO: Maybe add a method to Set to enumerate template names
// and use those instead.
- return nil, os.NewError("must specify names of top level templates")
+ return nil, &Error{ErrNoNames, "", 0, "must specify names of top level templates"}
}
e := escaper{
s,
@@ -49,10 +49,10 @@
for _, name := range names {
c, _ := e.escapeTree(context{}, name, 0)
var err os.Error
- if c.errStr != "" {
- err = fmt.Errorf("%s:%d: %s", name, c.errLine, c.errStr)
+ if c.err != nil {
+ err, c.err.Name = c.err, name
} else if c.state != stateText {
- err = fmt.Errorf("%s ends in a non-text context: %v", name, c)
+ err = &Error{ErrEndContext, name, 0, fmt.Sprintf("ends in a non-text context: %v", c)}
}
if err != nil {
// Prevent execution of unsafe templates.
@@ -163,9 +163,8 @@
s = append(s, "exp_template_html_urlescaper")
case urlPartUnknown:
return context{
- state: stateError,
- errLine: n.Line,
- errStr: fmt.Sprintf("%s appears in an ambiguous URL context", n),
+ state: stateError,
+ err: errorf(ErrAmbigContext, n.Line, "%s appears in an ambiguous URL context", n),
}
default:
panic(c.urlPart.String())
@@ -180,9 +179,8 @@
s = append(s, "exp_template_html_jsregexpescaper")
case stateComment, stateJSBlockCmt, stateJSLineCmt, stateCSSBlockCmt, stateCSSLineCmt:
return context{
- state: stateError,
- errLine: n.Line,
- errStr: fmt.Sprintf("%s appears inside a comment", n),
+ state: stateError,
+ err: errorf(ErrInsideComment, n.Line, "%s appears inside a comment", n),
}
case stateCSS:
s = append(s, "exp_template_html_cssvaluefilter")
@@ -319,9 +317,8 @@
}
return context{
- state: stateError,
- errLine: line,
- errStr: fmt.Sprintf("{{%s}} branches end in different contexts: %v, %v", nodeName, a, b),
+ state: stateError,
+ err: errorf(ErrBranchEnd, line, "{{%s}} branches end in different contexts: %v, %v", nodeName, a, b),
}
}
@@ -340,8 +337,8 @@
// 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
+ c0.err.Line = n.Line
+ c0.err.Description = "on range loop re-entry: " + c0.err.Description
return c0
}
}
@@ -386,9 +383,8 @@
t := e.template(name)
if t == nil {
return context{
- state: stateError,
- errStr: fmt.Sprintf("no such template %s", name),
- errLine: line,
+ state: stateError,
+ err: errorf(ErrNoSuchTemplate, line, "no such template %s", name),
}, dname
}
if dname != name {
@@ -428,8 +424,7 @@
d = context{
state: stateError,
// TODO: Find the first node with a line in t.Tree.Root
- errLine: 0,
- errStr: fmt.Sprintf("cannot compute output context for template %s", n),
+ err: errorf(ErrOutputContext, 0, "cannot compute output context for template %s", n),
}
// TODO: If necessary, compute a fixed point by assuming d
// as the input context, and recursing to escapeList with a
diff --git a/src/pkg/exp/template/html/escape_test.go b/src/pkg/exp/template/html/escape_test.go
index 0ab326c..4adf367 100644
--- a/src/pkg/exp/template/html/escape_test.go
+++ b/src/pkg/exp/template/html/escape_test.go
@@ -603,11 +603,11 @@
},
{
"<a b=1 c={{.H}}",
- "z ends in a non-text context: {stateAttr delimSpaceOrTagEnd",
+ "z: ends in a non-text context: {stateAttr delimSpaceOrTagEnd",
},
{
"<script>foo();",
- "z ends in a non-text context: {stateJS",
+ "z: ends in a non-text context: {stateJS",
},
{
`<a href="{{if .F}}/foo?a={{else}}/bar/{{end}}{{.H}}">`,
@@ -656,7 +656,7 @@
// 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: "/-"`,
+ `'/' could start div or regexp: "/-"`,
},
{
`{{template "foo"}}`,
@@ -666,7 +666,7 @@
`{{define "z"}}<div{{template "y"}}>{{end}}` +
// Illegal starting in stateTag but not in stateText.
`{{define "y"}} foo<b{{end}}`,
- `z:0: "<" in attribute name: " foo<b"`,
+ `"<" in attribute name: " foo<b"`,
},
{
`{{define "z"}}<script>reverseList = [{{template "t"}}]</script>{{end}}` +
@@ -701,7 +701,7 @@
continue
}
if strings.Index(got, test.err) == -1 {
- t.Errorf("input=%q: error %q does not contain expected string %q", test.input, got, test.err)
+ t.Errorf("input=%q: error\n\t%q\ndoes not contain expected string\n\t%q", test.input, got, test.err)
continue
}
}
diff --git a/src/pkg/exp/template/html/transition.go b/src/pkg/exp/template/html/transition.go
index 117b20a..2449a50 100644
--- a/src/pkg/exp/template/html/transition.go
+++ b/src/pkg/exp/template/html/transition.go
@@ -6,11 +6,12 @@
import (
"bytes"
- "fmt"
- "os"
"strings"
)
+// TODO: ensure transition error messages contain template name and ideally
+// line info.
+
// transitionFunc is the array of context transition functions for text nodes.
// A transition function takes a context and template text input, and returns
// the updated context and any unconsumed text.
@@ -82,8 +83,8 @@
i, err := eatAttrName(s, attrStart)
if err != nil {
return context{
- state: stateError,
- errStr: err.String(),
+ state: stateError,
+ err: err,
}, nil
}
if i == len(s) {
@@ -204,8 +205,8 @@
c.jsCtx = jsCtxRegexp
default:
return context{
- state: stateError,
- errStr: fmt.Sprintf("'/' could start div or regexp: %.32q", s[i:]),
+ state: stateError,
+ err: errorf(ErrSlashAmbig, 0, "'/' could start div or regexp: %.32q", s[i:]),
}, nil
}
default:
@@ -235,8 +236,8 @@
i++
if i == len(b) {
return context{
- state: stateError,
- errStr: fmt.Sprintf("unfinished escape sequence in JS string: %q", s),
+ state: stateError,
+ err: errorf(ErrPartialEscape, 0, "unfinished escape sequence in JS string: %q", s),
}, nil
}
} else {
@@ -271,8 +272,8 @@
i++
if i == len(b) {
return context{
- state: stateError,
- errStr: fmt.Sprintf("unfinished escape sequence in JS regexp: %q", s),
+ state: stateError,
+ err: errorf(ErrPartialEscape, 0, "unfinished escape sequence in JS regexp: %q", s),
}, nil
}
case '[':
@@ -289,8 +290,8 @@
// This can be fixed by making context richer if interpolation
// into charsets is desired.
return context{
- state: stateError,
- errStr: fmt.Sprintf("unfinished JS regexp charset: %q", s),
+ state: stateError,
+ err: errorf(ErrPartialCharset, 0, "unfinished JS regexp charset: %q", s),
}, nil
}
@@ -463,8 +464,8 @@
i++
if i == len(b) {
return context{
- state: stateError,
- errStr: fmt.Sprintf("unfinished escape sequence in CSS string: %q", s),
+ state: stateError,
+ err: errorf(ErrPartialEscape, 0, "unfinished escape sequence in CSS string: %q", s),
}, nil
}
} else {
@@ -486,7 +487,7 @@
// It returns an error if s[i:] does not look like it begins with an
// attribute name, such as encountering a quote mark without a preceding
// equals sign.
-func eatAttrName(s []byte, i int) (int, os.Error) {
+func eatAttrName(s []byte, i int) (int, *Error) {
for j := i; j < len(s); j++ {
switch s[j] {
case ' ', '\t', '\n', '\f', '\r', '=', '>':
@@ -495,7 +496,7 @@
// 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)
+ return -1, errorf(ErrBadHTML, 0, "%q in attribute name: %.32q", s[j:j+1], s)
default:
// No-op.
}
