text/template: perform value validity checks

Check reflect.Value.IsValid() before calling other reflect.Value methods
that panic on zero values.

Added tests for cases with untyped nils. They panicked without these fixes.

Removed a TODO.

Fixes #12356

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

diff --git a/src/text/template/exec.go b/src/text/template/exec.go
index 6e46d054a87a6ed4ffb99da164d528f8616cb28c..625e9b54d69f6d2441ca2e770a8dcd6293babba1 100644 (file)
--- a/src/text/template/exec.go
+++ b/src/text/template/exec.go
@@ -135,8 +135,6 @@ func errRecover(errp *error) {
                        *errp = err.Err // Strip the wrapper.
                case ExecError:
                        *errp = err // Keep the wrapper.
-               case error: // TODO: This should never happen, but it does. Understand and/or fix.
-                       *errp = err
                default:
                        panic(e)
                }

diff --git a/src/text/template/exec_test.go b/src/text/template/exec_test.go
index 9fd01320c2a5c00804b67d65ee312aaf8c08f924..b2ed8e79383db5b68fe4ce3e2a42c84b192dec12 100644 (file)
--- a/src/text/template/exec_test.go
+++ b/src/text/template/exec_test.go
@@ -337,6 +337,7 @@ var execTests = []execTest{
        {"if not .BinaryFunc call", "{{ if not .BinaryFunc}}{{call .BinaryFunc `1` `2`}}{{else}}No{{end}}", "No", tVal, true},
        {"Interface Call", `{{stringer .S}}`, "foozle", map[string]interface{}{"S": bytes.NewBufferString("foozle")}, true},
        {".ErrFunc", "{{call .ErrFunc}}", "bla", tVal, true},
+       {"call nil", "{{call nil}}", "", tVal, false},
 
        // Erroneous function calls (check args).
        {".BinaryFuncTooFew", "{{call .BinaryFunc `1`}}", "", tVal, false},
@@ -425,12 +426,15 @@ var execTests = []execTest{
        {"slice[1]", "{{index .SI 1}}", "4", tVal, true},
        {"slice[HUGE]", "{{index .SI 10}}", "", tVal, false},
        {"slice[WRONG]", "{{index .SI `hello`}}", "", tVal, false},
+       {"slice[nil]", "{{index .SI nil}}", "", tVal, false},
        {"map[one]", "{{index .MSI `one`}}", "1", tVal, true},
        {"map[two]", "{{index .MSI `two`}}", "2", tVal, true},
        {"map[NO]", "{{index .MSI `XXX`}}", "0", tVal, true},
-       {"map[nil]", "{{index .MSI nil}}", "0", tVal, true},
+       {"map[nil]", "{{index .MSI nil}}", "", tVal, false},
+       {"map[``]", "{{index .MSI ``}}", "0", tVal, true},
        {"map[WRONG]", "{{index .MSI 10}}", "", tVal, false},
        {"double index", "{{index .SMSI 1 `eleven`}}", "11", tVal, true},
+       {"nil[1]", "{{index nil 1}}", "", tVal, false},
 
        // Len.
        {"slice", "{{len .SI}}", "3", tVal, true},

diff --git a/src/text/template/funcs.go b/src/text/template/funcs.go
index ccd0dfc80d86b024a9759dddcdf82520cffc680b..be13ca2a3e35fda66ee1594baf7bf1b658f974cf 100644 (file)
--- a/src/text/template/funcs.go
+++ b/src/text/template/funcs.go
@@ -104,6 +104,21 @@ func findFunction(name string, tmpl *Template) (reflect.Value, bool) {
        return reflect.Value{}, false
 }
 
+// prepareArg checks if value can be used as an argument of type argType, and
+// converts an invalid value to appropriate zero if possible.
+func prepareArg(value reflect.Value, argType reflect.Type) (reflect.Value, error) {
+       if !value.IsValid() {
+               if !canBeNil(argType) {
+                       return reflect.Value{}, fmt.Errorf("value is nil; should be of type %s", argType)
+               }
+               value = reflect.Zero(argType)
+       }
+       if !value.Type().AssignableTo(argType) {
+               return reflect.Value{}, fmt.Errorf("value has type %s; should be %s", value.Type(), argType)
+       }
+       return value, nil
+}
+
 // Indexing.
 
 // index returns the result of indexing its first argument by the following
@@ -111,6 +126,9 @@ func findFunction(name string, tmpl *Template) (reflect.Value, bool) {
 // indexed item must be a map, slice, or array.
 func index(item interface{}, indices ...interface{}) (interface{}, error) {
        v := reflect.ValueOf(item)
+       if !v.IsValid() {
+               return nil, fmt.Errorf("index of untyped nil")
+       }
        for _, i := range indices {
                index := reflect.ValueOf(i)
                var isNil bool
@@ -125,6 +143,8 @@ func index(item interface{}, indices ...interface{}) (interface{}, error) {
                                x = index.Int()
                        case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
                                x = int64(index.Uint())
+                       case reflect.Invalid:
+                               return nil, fmt.Errorf("cannot index slice/array with nil")
                        default:
                                return nil, fmt.Errorf("cannot index slice/array with type %s", index.Type())
                        }
@@ -133,17 +153,18 @@ func index(item interface{}, indices ...interface{}) (interface{}, error) {
                        }
                        v = v.Index(int(x))
                case reflect.Map:
-                       if !index.IsValid() {
-                               index = reflect.Zero(v.Type().Key())
-                       }
-                       if !index.Type().AssignableTo(v.Type().Key()) {
-                               return nil, fmt.Errorf("%s is not index type for %s", index.Type(), v.Type())
+                       index, err := prepareArg(index, v.Type().Key())
+                       if err != nil {
+                               return nil, err
                        }
                        if x := v.MapIndex(index); x.IsValid() {
                                v = x
                        } else {
                                v = reflect.Zero(v.Type().Elem())
                        }
+               case reflect.Invalid:
+                       // the loop holds invariant: v.IsValid()
+                       panic("unreachable")
                default:
                        return nil, fmt.Errorf("can't index item of type %s", v.Type())
                }
@@ -155,7 +176,11 @@ func index(item interface{}, indices ...interface{}) (interface{}, error) {
 
 // length returns the length of the item, with an error if it has no defined length.
 func length(item interface{}) (int, error) {
-       v, isNil := indirect(reflect.ValueOf(item))
+       v := reflect.ValueOf(item)
+       if !v.IsValid() {
+               return 0, fmt.Errorf("len of untyped nil")
+       }
+       v, isNil := indirect(v)
        if isNil {
                return 0, fmt.Errorf("len of nil pointer")
        }
@@ -172,6 +197,9 @@ func length(item interface{}) (int, error) {
 // The function must return 1 result, or 2 results, the second of which is an error.
 func call(fn interface{}, args ...interface{}) (interface{}, error) {
        v := reflect.ValueOf(fn)
+       if !v.IsValid() {
+               return nil, fmt.Errorf("call of nil")
+       }
        typ := v.Type()
        if typ.Kind() != reflect.Func {
                return nil, fmt.Errorf("non-function of type %s", typ)
@@ -201,13 +229,11 @@ func call(fn interface{}, args ...interface{}) (interface{}, error) {
                } else {
                        argType = dddType
                }
-               if !value.IsValid() && canBeNil(argType) {
-                       value = reflect.Zero(argType)
-               }
-               if !value.Type().AssignableTo(argType) {
-                       return nil, fmt.Errorf("arg %d has type %s; should be %s", i, value.Type(), argType)
+
+               var err error
+               if argv[i], err = prepareArg(value, argType); err != nil {
+                       return nil, fmt.Errorf("arg %d: %s", i, err)
                }
-               argv[i] = value
        }
        result := v.Call(argv)
        if len(result) == 2 && !result[1].IsNil() {