From 07ee3cc741604136254499ccaf1e6c9d1bd868ff Mon Sep 17 00:00:00 2001 From: Rob Pike Date: Wed, 30 Nov 2011 17:42:18 -0500 Subject: [PATCH] html/template: update to new template API Not quite done yet but enough is here to review. Embedding is eliminated so clients can't accidentally reach methods of text/template.Template that would break the invariants. TODO later: Add and Clone are unimplemented. TODO later: address issue 2349 R=golang-dev, r, rsc CC=golang-dev https://golang.org/cl/5434077 --- src/pkg/Makefile | 1 + src/pkg/html/template/clone_test.go | 34 +-- src/pkg/html/template/escape.go | 47 ++-- src/pkg/html/template/escape_test.go | 38 ++-- src/pkg/html/template/template.go | 327 +++++++++++++-------------- 5 files changed, 213 insertions(+), 234 deletions(-) diff --git a/src/pkg/Makefile b/src/pkg/Makefile index 84399bdafc..12930d6a18 100644 --- a/src/pkg/Makefile +++ b/src/pkg/Makefile @@ -102,6 +102,7 @@ DIRS=\ hash/crc64\ hash/fnv\ html\ + html/template\ image\ image/bmp\ image/color\ diff --git a/src/pkg/html/template/clone_test.go b/src/pkg/html/template/clone_test.go index ed1698acd8..39788173b9 100644 --- a/src/pkg/html/template/clone_test.go +++ b/src/pkg/html/template/clone_test.go @@ -7,8 +7,6 @@ package template import ( "bytes" "testing" - "text/template" - "text/template/parse" ) func TestClone(t *testing.T) { @@ -48,15 +46,20 @@ func TestClone(t *testing.T) { } for _, test := range tests { - s := template.Must(template.New("s").Parse(test.input)) - d := template.New("d") - d.Tree = &parse.Tree{Name: d.Name(), Root: cloneList(s.Root)} + s, err := New("s").Parse(test.input) + if err != nil { + t.Errorf("input=%q: unexpected parse error %v", test.input, err) + } - if want, got := s.Root.String(), d.Root.String(); want != got { + d, _ := New("d").Parse(test.input) + // Hack: just replace the root of the tree. + d.text.Root = cloneList(s.text.Root) + + if want, got := s.text.Root.String(), d.text.Root.String(); want != got { t.Errorf("want %q, got %q", want, got) } - err := escape(d) + err = escapeTemplates(d, "d") if err != nil { t.Errorf("%q: failed to escape: %s", test.input, err) continue @@ -73,18 +76,17 @@ func TestClone(t *testing.T) { data := []string{"foo", "", "baz"} - // Make sure escaping d did not affect s. var b bytes.Buffer - s.Execute(&b, data) - if got := b.String(); got != test.want { - t.Errorf("%q: want %q, got %q", test.input, test.want, got) - continue - } - - b.Reset() d.Execute(&b, data) if got := b.String(); got != test.wantClone { - t.Errorf("%q: want %q, got %q", test.input, test.wantClone, got) + t.Errorf("input=%q: want %q, got %q", test.input, test.wantClone, got) + } + + // Make sure escaping d did not affect s. + b.Reset() + s.text.Execute(&b, data) + if got := b.String(); got != test.want { + t.Errorf("input=%q: want %q, got %q", test.input, test.want, got) } } } diff --git a/src/pkg/html/template/escape.go b/src/pkg/html/template/escape.go index 8ac07eae24..501aef970b 100644 --- a/src/pkg/html/template/escape.go +++ b/src/pkg/html/template/escape.go @@ -12,24 +12,15 @@ import ( "text/template/parse" ) -// escape rewrites each action in the template to guarantee that the output is -// properly escaped. -func escape(t *template.Template) error { - var s template.Set - s.Add(t) - return escapeSet(&s, t.Name()) - // TODO: if s contains cloned dependencies due to self-recursion - // cross-context, error out. -} - -// escapeSet rewrites the template set to guarantee that the output of any of -// the named templates is properly escaped. -// Names should include the names of all templates that might be Executed but -// need not include helper templates. -// If no error is returned, then the named templates have been modified. -// Otherwise the named templates have been rendered unusable. -func escapeSet(s *template.Set, names ...string) error { - e := newEscaper(s) +// escapeTemplates rewrites the named templates, which must be +// associated with t, to guarantee that the output of any of the named +// templates is properly escaped. Names should include the names of +// all templates that might be Executed but need not include helper +// templates. If no error is returned, then the named templates have +// been modified. Otherwise the named templates have been rendered +// unusable. +func escapeTemplates(tmpl *Template, names ...string) error { + e := newEscaper(tmpl) for _, name := range names { c, _ := e.escapeTree(context{}, name, 0) var err error @@ -41,12 +32,13 @@ func escapeSet(s *template.Set, names ...string) error { if err != nil { // Prevent execution of unsafe templates. for _, name := range names { - if t := s.Template(name); t != nil { - t.Tree = nil + if t := tmpl.Lookup(name); t != nil { + t.text.Tree = nil } } return err } + tmpl.escaped = true } e.commit() return nil @@ -83,8 +75,7 @@ var equivEscapers = map[string]string{ // escaper collects type inferences about templates and changes needed to make // templates injection safe. type escaper struct { - // set is the template set being escaped. - set *template.Set + tmpl *Template // output[templateName] is the output context for a templateName that // has been mangled to include its input context. output map[string]context @@ -102,9 +93,9 @@ type escaper struct { } // newEscaper creates a blank escaper for the given set. -func newEscaper(s *template.Set) *escaper { +func newEscaper(t *Template) *escaper { return &escaper{ - s, + t, map[string]context{}, map[string]*template.Template{}, map[string]bool{}, @@ -442,7 +433,7 @@ func (e *escaper) escapeList(c context, n *parse.ListNode) context { // It returns the best guess at an output context, and the result of the filter // which is the same as whether e was updated. func (e *escaper) escapeListConditionally(c context, n *parse.ListNode, filter func(*escaper, context) bool) (context, bool) { - e1 := newEscaper(e.set) + e1 := newEscaper(e.tmpl) // Make type inferences available to f. for k, v := range e.output { e1.output[k] = v @@ -501,7 +492,7 @@ func (e *escaper) escapeTree(c context, name string, line int) (context, string) }, dname } if dname != name { - // Use any template derived during an earlier call to escapeSet + // Use any template derived during an earlier call to escapeTemplate // with different top level templates, or clone if necessary. dt := e.template(dname) if dt == nil { @@ -729,7 +720,7 @@ func (e *escaper) commit() { e.template(name).Funcs(funcMap) } for _, t := range e.derived { - e.set.Add(t) + e.tmpl.text.Add(t) } for n, s := range e.actionNodeEdits { ensurePipelineContains(n.Pipe, s) @@ -744,7 +735,7 @@ func (e *escaper) commit() { // template returns the named template given a mangled template name. func (e *escaper) template(name string) *template.Template { - t := e.set.Template(name) + t := e.tmpl.text.Lookup(name) if t == nil { t = e.derived[name] } diff --git a/src/pkg/html/template/escape_test.go b/src/pkg/html/template/escape_test.go index 4af583097b..b4daca7d6b 100644 --- a/src/pkg/html/template/escape_test.go +++ b/src/pkg/html/template/escape_test.go @@ -806,13 +806,15 @@ func TestEscapeSet(t *testing.T) { for name, body := range test.inputs { source += fmt.Sprintf("{{define %q}}%s{{end}} ", name, body) } - s := &Set{} - s.Funcs(fns) - s.Parse(source) + tmpl, err := New("root").Funcs(fns).Parse(source) + if err != nil { + t.Errorf("error parsing %q: %v", source, err) + continue + } var b bytes.Buffer - if err := s.Execute(&b, "main", data); err != nil { - t.Errorf("%q executing %v", err.Error(), s.Template("main")) + if err := tmpl.ExecuteTemplate(&b, "main", data); err != nil { + t.Errorf("%q executing %v", err.Error(), tmpl.Lookup("main")) continue } if got := b.String(); test.want != got { @@ -929,13 +931,13 @@ func TestErrors(t *testing.T) { "z:1: no such template foo", }, { - `{{define "z"}}{{end}}` + + `` + // Illegal starting in stateTag but not in stateText. `{{define "y"}} fooreverseList = [{{template "t"}}]{{end}}` + + `` + // Missing " after recursive call. `{{define "t"}}{{if .Tail}}{{template "t" .Tail}}{{end}}{{.Head}}",{{end}}`, `: cannot compute output context for template t$htmltemplate_stateJS_elementScript`, @@ -967,21 +969,13 @@ func TestErrors(t *testing.T) { } for _, test := range tests { - var err error buf := new(bytes.Buffer) - if strings.HasPrefix(test.input, "{{define") { - var s *Set - s, err = (&Set{}).Parse(test.input) - if err == nil { - err = s.Execute(buf, "z", nil) - } - } else { - var t *Template - t, err = New("z").Parse(test.input) - if err == nil { - err = t.Execute(buf, nil) - } + tmpl, err := New("z").Parse(test.input) + if err != nil { + t.Errorf("input=%q: unexpected parse error %s\n", test.input, err) + continue } + err = tmpl.Execute(buf, nil) var got string if err != nil { got = err.Error() @@ -1569,11 +1563,11 @@ func TestEscapeErrorsNotIgnorable(t *testing.T) { func TestEscapeSetErrorsNotIgnorable(t *testing.T) { var b bytes.Buffer - s, err := (&Set{}).Parse(`{{define "t"}}