From ca91de7ca09fef7ca000fc0267918c8fdd34b429 Mon Sep 17 00:00:00 2001 From: Didier Spezia Date: Mon, 8 Jun 2015 20:04:14 +0000 Subject: [PATCH] html/template: prevent panic while escaping pipelines AFAIK, the documentation does not explicitly state whether variables can store a callable entity or not. I believe the current implementation in text/template assumes they cannot though. The call builtin function is supposed to be used for this purpose. Template "{{0|$}}" should generate an error at runtime, instead of a panic. Similarly, template "{{0|(nil)}}" should not generate a panic. This CL aborts the sanitization process for a given pipeline when no identifier can be derived from the selected node. It happens with malformed pipelines. We now have the following errors: {{ 0 | $ }} template: foo:1:10: executing "foo" at <$>: can't give argument to non-function $ {{ 0 | (nil) }} template: foo:1:11: executing "foo" at : nil is not a command Fixes #11118 Fixes #11356 Change-Id: Idae52f806849f4c9ab7aca1b4bb4b59a74723d0e Reviewed-on: https://go-review.googlesource.com/10823 Reviewed-by: Rob Pike --- src/html/template/escape.go | 2 +- src/html/template/escape_test.go | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/html/template/escape.go b/src/html/template/escape.go index bfcea66b90..3c18340547 100644 --- a/src/html/template/escape.go +++ b/src/html/template/escape.go @@ -215,7 +215,7 @@ func allIdents(node parse.Node) []string { case *parse.ChainNode: return node.Field } - panic("unidentified node type in allIdents") + return nil } // ensurePipelineContains ensures that the pipeline has commands with diff --git a/src/html/template/escape_test.go b/src/html/template/escape_test.go index 41ab0c8ae7..bea2d133c3 100644 --- a/src/html/template/escape_test.go +++ b/src/html/template/escape_test.go @@ -1586,6 +1586,28 @@ func TestEnsurePipelineContains(t *testing.T) { } } +func TestEscapeMalformedPipelines(t *testing.T) { + tests := []string{ + "{{ 0 | $ }}", + "{{ 0 | $ | urlquery }}", + "{{ 0 | $ | urlquery | html }}", + "{{ 0 | (nil) }}", + "{{ 0 | (nil) | html }}", + "{{ 0 | (nil) | html | urlquery }}", + } + for _, test := range tests { + var b bytes.Buffer + tmpl, err := New("test").Parse(test) + if err != nil { + t.Errorf("failed to parse set: %q", err) + } + err = tmpl.Execute(&b, nil) + if err == nil { + t.Errorf("Expected error for %q", test) + } + } +} + func TestEscapeErrorsNotIgnorable(t *testing.T) { var b bytes.Buffer tmpl, _ := New("dangerous").Parse("