From: Nodir Turakulov Date: Sun, 6 Sep 2015 06:16:49 +0000 (-0700) Subject: text/template: perform value validity checks X-Git-Tag: go1.6beta1~1127 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6599450016244b9e3e074e87d7064219ee2e5cf8;p=gostls13.git 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 --- diff --git a/src/text/template/exec.go b/src/text/template/exec.go index 6e46d054a8..625e9b54d6 100644 --- 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 9fd01320c2..b2ed8e7938 100644 --- 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 ccd0dfc80d..be13ca2a3e 100644 --- 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() {