From d69588131c63571aa109db652e373b4815635c63 Mon Sep 17 00:00:00 2001 From: Michael Gaffney Date: Sun, 28 Jun 2020 16:55:57 -0400 Subject: [PATCH] Fix for panic in db.setFieldsToNil (#155) --- internal/db/read_writer.go | 91 +++--- internal/db/read_writer_test.go | 488 ++++++++++++++++++++++++++++++++ 2 files changed, 543 insertions(+), 36 deletions(-) diff --git a/internal/db/read_writer.go b/internal/db/read_writer.go index 3b2ddc1a5..b408e840d 100644 --- a/internal/db/read_writer.go +++ b/internal/db/read_writer.go @@ -613,43 +613,9 @@ func filterPaths(paths []string) []string { return filtered } -// setFieldsToNil will set the fieldNames to nil. It supports embedding a struct -// (or structPtr) one level deep. func setFieldsToNil(i interface{}, fieldNames []string) { - val := reflect.Indirect(reflect.ValueOf(i)) - structTyp := val.Type() - for _, field := range fieldNames { - for i := 0; i < structTyp.NumField(); i++ { - kind := structTyp.Field(i).Type.Kind() - if kind == reflect.Struct || kind == reflect.Ptr { - // check if the embedded field is exported via CanInterface() - if val.Field(i).CanInterface() { - embVal := reflect.Indirect(reflect.ValueOf(val.Field(i).Interface())) - // if it's a ptr to a struct, then we need a few more bits before proceeding. - if kind == reflect.Ptr { - embVal = val.Field(i).Elem() - embType := embVal.Type() - if embType.Kind() != reflect.Struct { - continue - } - } - f := embVal.FieldByName(field) - if f.IsValid() { - if f.CanSet() { - f.Set(reflect.Zero(f.Type())) - } - } - continue - } - } - // it's not an embedded type, so check if the field name matches - f := val.FieldByName(field) - if f.IsValid() { - if f.CanSet() { - f.Set(reflect.Zero(f.Type())) - } - } - } + if err := Clear(i, fieldNames, 2); err != nil { + // do nothing } } @@ -672,3 +638,56 @@ func contains(ss []string, t string) bool { } return false } + +// Clear sets fields in the value pointed to by i to their zero value. +// Clear descends i to depth clearing fields at each level. i must be a +// pointer to a struct. Cycles in i are not detected. +// +// A depth of 2 will change i and i's children. A depth of 1 will change i +// but no children of i. A depth of 0 will return with no changes to i. +func Clear(i interface{}, fields []string, depth int) error { + if len(fields) == 0 || depth == 0 { + return nil + } + fm := make(map[string]bool) + for _, f := range fields { + fm[f] = true + } + + v := reflect.ValueOf(i) + + switch v.Kind() { + default: + return ErrInvalidParameter + case reflect.Ptr: + if v.IsNil() || v.Elem().Kind() != reflect.Struct { + return ErrInvalidParameter + } + clear(v, fm, depth) + } + return nil +} + +func clear(v reflect.Value, fields map[string]bool, depth int) { + if depth == 0 { + return + } + depth-- + + switch v.Kind() { + case reflect.Ptr: + clear(v.Elem(), fields, depth+1) + case reflect.Struct: + typeOfT := v.Type() + for i := 0; i < v.NumField(); i++ { + f := v.Field(i) + if ok := fields[typeOfT.Field(i).Name]; ok { + if f.IsValid() && f.CanSet() { + f.Set(reflect.Zero(f.Type())) + } + continue + } + clear(f, fields, depth) + } + } +} diff --git a/internal/db/read_writer_test.go b/internal/db/read_writer_test.go index 614a65a2f..4ebfbdadc 100644 --- a/internal/db/read_writer_test.go +++ b/internal/db/read_writer_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "strings" "testing" "time" @@ -1285,3 +1286,490 @@ func testId(t *testing.T) string { assert.NoError(err) return id } + +func TestClear_InputTypes(t *testing.T) { + type Z struct { + F string + } + + var nilZ *Z + s := "test-string" + + type args struct { + v interface{} + f []string + d int + } + + var tests = []struct { + name string + args args + want interface{} + err error + }{ + { + name: "nil", + args: args{ + v: nil, + f: []string{"field"}, + d: 1, + }, + err: ErrInvalidParameter, + }, + { + name: "string", + args: args{ + v: "blank", + f: []string{"field"}, + d: 1, + }, + err: ErrInvalidParameter, + }, + { + name: "pointer-to-nil-struct", + args: args{ + v: nilZ, + f: []string{"field"}, + d: 1, + }, + err: ErrInvalidParameter, + }, + { + name: "pointer-to-string", + args: args{ + v: &s, + f: []string{"field"}, + d: 1, + }, + err: ErrInvalidParameter, + }, + { + name: "not-pointer", + args: args{ + v: Z{ + F: "foo", + }, + f: []string{"field"}, + d: 1, + }, + err: ErrInvalidParameter, + }, + { + name: "map", + args: args{ + v: map[string]int{ + "A": 31, + "B": 34, + }, + f: []string{"field"}, + d: 1, + }, + err: ErrInvalidParameter, + }, + { + name: "pointer-to-struct", + args: args{ + v: &Z{ + F: "foo", + }, + f: []string{"field"}, + d: 1, + }, + want: &Z{ + F: "foo", + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + input := tt.args.v + err := Clear(input, tt.args.f, tt.args.d) + if tt.err != nil { + assert.Error(err) + return + } + require.NoError(err) + assert.Equal(tt.want, input) + }) + } +} + +func TestClear_Structs(t *testing.T) { + s := "test-string" + + type A struct{ F string } + type B struct{ F *string } + + type AB struct { + A A + B *B + F string + + IN io.Reader + MP map[int]int + SL []string + AR [1]int + CH chan int + FN func() + } + + type AFB struct { + F A + B *B + } + + type ABF struct { + A A + F *B + } + + type C struct { + F string + NF string + } + + type EA struct { + A + M string + NF string + } + + type EAP struct { + *A + M string + NF string + } + + type DEA struct { + EA + F string + } + + type DEAP struct { + *EAP + F string + } + + type args struct { + v interface{} + f []string + d int + } + var tests = []struct { + name string + args args + want interface{} + }{ + { + name: "blank-A", + args: args{ + v: &A{}, + f: []string{"F"}, + d: 1, + }, + want: &A{}, + }, + { + name: "clear-A", + args: args{ + v: &A{"clear"}, + f: []string{"F"}, + d: 1, + }, + want: &A{}, + }, + { + name: "clear-B", + args: args{ + v: &B{&s}, + f: []string{"F"}, + d: 1, + }, + want: &B{}, + }, + { + name: "clear-C", + args: args{ + v: &C{"clear", "notclear"}, + f: []string{"F"}, + d: 1, + }, + want: &C{"", "notclear"}, + }, + { + name: "shallow-clear-AB", + args: args{ + v: &AB{ + A: A{"notclear"}, + B: &B{&s}, + F: "clear", + }, + f: []string{"F"}, + d: 1, + }, + want: &AB{ + A: A{"notclear"}, + B: &B{&s}, + F: "", + }, + }, + { + name: "deep-clear-AB", + args: args{ + v: &AB{ + A: A{"clear"}, + B: &B{&s}, + F: "clear", + }, + f: []string{"F"}, + d: 2, + }, + want: &AB{ + A: A{""}, + B: &B{}, + F: "", + }, + }, + { + name: "clear-AFB", + args: args{ + v: &AFB{ + F: A{"clear"}, + B: &B{&s}, + }, + f: []string{"F"}, + d: 2, + }, + want: &AFB{ + F: A{""}, + B: &B{}, + }, + }, + { + name: "clear-ABF", + args: args{ + v: &ABF{ + A: A{"clear"}, + F: &B{&s}, + }, + f: []string{"F"}, + d: 2, + }, + want: &ABF{ + A: A{""}, + F: nil, + }, + }, + { + name: "embedded-struct", + args: args{ + v: &EA{ + A: A{"clear"}, + M: "clear", + NF: "notclear", + }, + f: []string{"F", "M"}, + d: 2, + }, + want: &EA{ + A: A{""}, + M: "", + NF: "notclear", + }, + }, + { + name: "embedded-struct-pointer", + args: args{ + v: &EAP{ + A: &A{"clear"}, + M: "clear", + NF: "notclear", + }, + f: []string{"F", "M"}, + d: 2, + }, + want: &EAP{ + A: &A{""}, + M: "", + NF: "notclear", + }, + }, + { + name: "embedded-struct-pointer-extra-depth", + args: args{ + v: &EAP{ + A: &A{"clear"}, + M: "clear", + NF: "notclear", + }, + f: []string{"F", "M"}, + d: 12, + }, + want: &EAP{ + A: &A{""}, + M: "", + NF: "notclear", + }, + }, + { + name: "deep-embedded-struct", + args: args{ + v: &DEA{ + EA: EA{ + A: A{"clear"}, + M: "clear", + NF: "notclear", + }, + F: "clear", + }, + f: []string{"F", "M"}, + d: 3, + }, + want: &DEA{ + EA: EA{ + A: A{""}, + M: "", + NF: "notclear", + }, + F: "", + }, + }, + { + name: "deep-embedded-struct-pointer", + args: args{ + v: &DEAP{ + EAP: &EAP{ + A: &A{"clear"}, + M: "clear", + NF: "notclear", + }, + F: "clear", + }, + f: []string{"F", "M"}, + d: 3, + }, + want: &DEAP{ + EAP: &EAP{ + A: &A{""}, + M: "", + NF: "notclear", + }, + F: "", + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + input := tt.args.v + err := Clear(input, tt.args.f, tt.args.d) + assert.NotEmpty(s) + require.NoError(err) + assert.Equal(tt.want, input) + }) + } +} + +func TestClear_SetFieldsToNil(t *testing.T) { + type P struct{ F *string } + type A struct{ F string } + + type EA struct { + A + M string + NF string + } + + type EAP struct { + *A + M string + NF string + } + + type DEA struct { + EA + F string + } + + type DEAP struct { + *EAP + F string + } + + type args struct { + v interface{} + f []string + } + var tests = []struct { + name string + args args + want interface{} + }{ + { + name: "dont-panic", + args: args{ + v: P{}, + f: []string{"F"}, + }, + want: P{}, + }, + { + name: "deep-embedded-struct", + args: args{ + v: &DEA{ + EA: EA{ + A: A{"notclear"}, + M: "clear", + NF: "notclear", + }, + F: "clear", + }, + f: []string{"F", "M"}, + }, + want: &DEA{ + EA: EA{ + A: A{"notclear"}, + M: "", + NF: "notclear", + }, + F: "", + }, + }, + { + name: "deep-embedded-struct-pointer", + args: args{ + v: &DEAP{ + EAP: &EAP{ + A: &A{"notclear"}, + M: "clear", + NF: "notclear", + }, + F: "clear", + }, + f: []string{"F", "M"}, + }, + want: &DEAP{ + EAP: &EAP{ + A: &A{"notclear"}, + M: "", + NF: "notclear", + }, + F: "", + }, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + assert, require := assert.New(t), require.New(t) + input := tt.args.v + require.NotPanics(func() { + setFieldsToNil(input, tt.args.f) + }) + assert.Equal(tt.want, input) + }) + } +}