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>: nil is not a command

Fixes #11118
Fixes #11356

Change-Id: Idae52f806849f4c9ab7aca1b4bb4b59a74723d0e
Reviewed-on: https://go-review.googlesource.com/10823
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/html/template/escape.go b/src/html/template/escape.go
index bfcea66b905c63d972c231a454eb39253567084e..3c183405478ab5e8b49411256a76b6a0b5202a7e 100644 (file)
--- 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 41ab0c8ae7bac62166ac4a6a9cfb3fcf0eaf6d28..bea2d133c337a1220220e891e9e7cf6d85acab08 100644 (file)
--- 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("<a")