Commit 96f9e883 authored by Mike Samuel's avatar Mike Samuel

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
parent 642d272c
......@@ -11,6 +11,7 @@ GOFILES=\
context.go\
css.go\
doc.go\
error.go\
escape.go\
html.go\
js.go\
......
......@@ -21,8 +21,7 @@ type context struct {
urlPart urlPart
jsCtx jsCtx
element element
errLine int
errStr string
err *Error
}
// eq returns whether two contexts are equal.
......@@ -32,8 +31,7 @@ func (c context) eq(d context) bool {
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
......
......@@ -69,178 +69,8 @@ in this case,
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.
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 {{...}}.
See the documentation of ErrorCode for details.
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 @@ details necessary to understand escaping contexts and error messages. Most users
will not need to understand these details.
Contexts
Assuming {{.}} is `O'Reilly: How are <i>you</i>?`, the table below shows
......
// 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...)}
}
......@@ -36,7 +36,7 @@ func EscapeSet(s *template.Set, names ...string) (*template.Set, os.Error) {
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 @@ func EscapeSet(s *template.Set, names ...string) (*template.Set, os.Error) {
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 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
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 @@ func (e *escaper) escapeAction(c context, n *parse.ActionNode) context {
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 @@ func join(a, b context, line int, nodeName string) context {
}
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 @@ func (e *escaper) escapeBranch(c context, n *parse.BranchNode, nodeName string)
// 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 @@ func (e *escaper) escapeTree(c context, name string, line int) (context, string)
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 @@ func (e *escaper) computeOutCtx(c context, t *template.Template) context {
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
......
......@@ -603,11 +603,11 @@ func TestErrors(t *testing.T) {
},
{
"<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 @@ func TestErrors(t *testing.T) {
// 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 @@ func TestErrors(t *testing.T) {
`{{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 @@ func TestErrors(t *testing.T) {
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
}
}
......
......@@ -6,11 +6,12 @@ package html
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 @@ func tTag(c context, s []byte) (context, []byte) {
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 @@ func tJS(c context, s []byte) (context, []byte) {
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 @@ func tJSStr(c context, s []byte) (context, []byte) {
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 @@ func tJSRegexp(c context, s []byte) (context, []byte) {
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 @@ func tJSRegexp(c context, s []byte) (context, []byte) {
// 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 @@ func tCSSStr(c context, s []byte) (context, []byte) {
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 @@ func tError(c context, s []byte) (context, []byte) {
// 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 @@ func eatAttrName(s []byte, i int) (int, os.Error) {
// 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.
}
......
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