From 58a4c3b16bb7d54546432e047a86e52d8497ae72 Mon Sep 17 00:00:00 2001 From: Ryan Tinianov Date: Mon, 30 Oct 2023 11:52:25 -0400 Subject: [PATCH 1/3] Fix GH-340: Decoding array of slices causes panic --- mapstructure.go | 2 +- mapstructure_bugs_test.go | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/mapstructure.go b/mapstructure.go index 7581806a..1f646317 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -1163,7 +1163,7 @@ func (d *Decoder) decodeArray(name string, data interface{}, val reflect.Value) valArray := val - if valArray.Interface() == reflect.Zero(valArray.Type()).Interface() || d.config.ZeroFields { + if valArray.Comparable() && valArray.Interface() == reflect.Zero(valArray.Type()).Interface() || d.config.ZeroFields { // Check input type if dataValKind != reflect.Array && dataValKind != reflect.Slice { if d.config.WeaklyTypedInput { diff --git a/mapstructure_bugs_test.go b/mapstructure_bugs_test.go index 77bb3132..df44b9d9 100644 --- a/mapstructure_bugs_test.go +++ b/mapstructure_bugs_test.go @@ -625,3 +625,19 @@ func TestMapOmitEmptyWithEmptyFieldnameInTag(t *testing.T) { t.Fatalf("fail: %#v", m) } } + +// GH-340: Decoding array of slices causes panic +type HasNonComparableType struct { + NonComparableType [2][]byte +} + +func TestDecode_nonComparableType(t *testing.T) { + decodeTo := &HasNonComparableType{} + expected := [2][]byte{{1, 2}, {3, 4, 5}} + if err := Decode(map[string]interface{}{"NonComparableType": expected}, &decodeTo); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(expected, decodeTo.NonComparableType) { + t.Fatalf("fail: %#v", decodeTo.NonComparableType) + } +} From 64e322c04609ae490c334e7299694bff317ffb0f Mon Sep 17 00:00:00 2001 From: Fabrice Vaillant Date: Fri, 14 Apr 2023 11:41:37 +0200 Subject: [PATCH 2/3] Also precompute hook in decoder --- mapstructure.go | 126 +++++++++++++++++++++++++----------------------- 1 file changed, 65 insertions(+), 61 deletions(-) diff --git a/mapstructure.go b/mapstructure.go index 7581806a..7e92cab5 100644 --- a/mapstructure.go +++ b/mapstructure.go @@ -9,84 +9,84 @@ // // The simplest function to start with is Decode. // -// Field Tags +// # Field Tags // // When decoding to a struct, mapstructure will use the field name by // default to perform the mapping. For example, if a struct has a field // "Username" then mapstructure will look for a key in the source value // of "username" (case insensitive). // -// type User struct { -// Username string -// } +// type User struct { +// Username string +// } // // You can change the behavior of mapstructure by using struct tags. // The default struct tag that mapstructure looks for is "mapstructure" // but you can customize it using DecoderConfig. // -// Renaming Fields +// # Renaming Fields // // To rename the key that mapstructure looks for, use the "mapstructure" // tag and set a value directly. For example, to change the "username" example // above to "user": // -// type User struct { -// Username string `mapstructure:"user"` -// } +// type User struct { +// Username string `mapstructure:"user"` +// } // -// Embedded Structs and Squashing +// # Embedded Structs and Squashing // // Embedded structs are treated as if they're another field with that name. // By default, the two structs below are equivalent when decoding with // mapstructure: // -// type Person struct { -// Name string -// } +// type Person struct { +// Name string +// } // -// type Friend struct { -// Person -// } +// type Friend struct { +// Person +// } // -// type Friend struct { -// Person Person -// } +// type Friend struct { +// Person Person +// } // // This would require an input that looks like below: // -// map[string]interface{}{ -// "person": map[string]interface{}{"name": "alice"}, -// } +// map[string]interface{}{ +// "person": map[string]interface{}{"name": "alice"}, +// } // // If your "person" value is NOT nested, then you can append ",squash" to // your tag value and mapstructure will treat it as if the embedded struct // were part of the struct directly. Example: // -// type Friend struct { -// Person `mapstructure:",squash"` -// } +// type Friend struct { +// Person `mapstructure:",squash"` +// } // // Now the following input would be accepted: // -// map[string]interface{}{ -// "name": "alice", -// } +// map[string]interface{}{ +// "name": "alice", +// } // // When decoding from a struct to a map, the squash tag squashes the struct // fields into a single map. Using the example structs from above: // -// Friend{Person: Person{Name: "alice"}} +// Friend{Person: Person{Name: "alice"}} // // Will be decoded into a map: // -// map[string]interface{}{ -// "name": "alice", -// } +// map[string]interface{}{ +// "name": "alice", +// } // // DecoderConfig has a field that changes the behavior of mapstructure // to always squash embedded structs. // -// Remainder Values +// # Remainder Values // // If there are any unmapped keys in the source value, mapstructure by // default will silently ignore them. You can error by setting ErrorUnused @@ -98,20 +98,20 @@ // probably be a "map[string]interface{}" or "map[interface{}]interface{}". // See example below: // -// type Friend struct { -// Name string -// Other map[string]interface{} `mapstructure:",remain"` -// } +// type Friend struct { +// Name string +// Other map[string]interface{} `mapstructure:",remain"` +// } // // Given the input below, Other would be populated with the other // values that weren't used (everything but "name"): // -// map[string]interface{}{ -// "name": "bob", -// "address": "123 Maple St.", -// } +// map[string]interface{}{ +// "name": "bob", +// "address": "123 Maple St.", +// } // -// Omit Empty Values +// # Omit Empty Values // // When decoding from a struct to any other value, you may use the // ",omitempty" suffix on your tag to omit that value if it equates to @@ -122,37 +122,37 @@ // field value is zero and a numeric type, the field is empty, and it won't // be encoded into the destination type. // -// type Source struct { -// Age int `mapstructure:",omitempty"` -// } +// type Source struct { +// Age int `mapstructure:",omitempty"` +// } // -// Unexported fields +// # Unexported fields // // Since unexported (private) struct fields cannot be set outside the package // where they are defined, the decoder will simply skip them. // // For this output type definition: // -// type Exported struct { -// private string // this unexported field will be skipped -// Public string -// } +// type Exported struct { +// private string // this unexported field will be skipped +// Public string +// } // // Using this map as input: // -// map[string]interface{}{ -// "private": "I will be ignored", -// "Public": "I made it through!", -// } +// map[string]interface{}{ +// "private": "I will be ignored", +// "Public": "I made it through!", +// } // // The following struct will be decoded: // -// type Exported struct { -// private: "" // field is left with an empty string (zero value) -// Public: "I made it through!" -// } +// type Exported struct { +// private: "" // field is left with an empty string (zero value) +// Public: "I made it through!" +// } // -// Other Configuration +// # Other Configuration // // mapstructure is highly configurable. See the DecoderConfig struct // for other features and options that are supported. @@ -282,7 +282,8 @@ type DecoderConfig struct { // structure. The top-level Decode method is just a convenience that sets // up the most basic Decoder. type Decoder struct { - config *DecoderConfig + config *DecoderConfig + cachedDecodeHook func(from reflect.Value, to reflect.Value) (interface{}, error) } // Metadata contains information about decoding a structure that @@ -407,6 +408,9 @@ func NewDecoder(config *DecoderConfig) (*Decoder, error) { result := &Decoder{ config: config, } + if config.DecodeHook != nil { + result.cachedDecodeHook = cachedDecodeHook(config.DecodeHook) + } return result, nil } @@ -453,10 +457,10 @@ func (d *Decoder) decode(name string, input interface{}, outVal reflect.Value) e return nil } - if d.config.DecodeHook != nil { + if d.cachedDecodeHook != nil { // We have a DecodeHook, so let's pre-process the input. var err error - input, err = DecodeHookExec(d.config.DecodeHook, inputVal, outVal) + input, err = d.cachedDecodeHook(inputVal, outVal) if err != nil { return fmt.Errorf("error decoding '%s': %w", name, err) } From 859cdb58054b1734a49659b00bdc928d4a2833ba Mon Sep 17 00:00:00 2001 From: Fabrice Vaillant Date: Fri, 14 Apr 2023 09:36:43 +0200 Subject: [PATCH 3/3] Precompute decode hooks for performance reason --- decode_hooks.go | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/decode_hooks.go b/decode_hooks.go index c1f99da0..064e7baf 100644 --- a/decode_hooks.go +++ b/decode_hooks.go @@ -35,6 +35,30 @@ func typedDecodeHook(h DecodeHookFunc) DecodeHookFunc { return nil } +// cachedDecodeHook takes a raw DecodeHookFunc (an interface{}) and turns +// it into a closure to be used directly +// if the type fails to convert we return a closure always erroring to keep the previous behaviour +func cachedDecodeHook(raw DecodeHookFunc) func(from reflect.Value, to reflect.Value) (interface{}, error) { + switch f := typedDecodeHook(raw).(type) { + case DecodeHookFuncType: + return func(from reflect.Value, to reflect.Value) (interface{}, error) { + return f(from.Type(), to.Type(), from.Interface()) + } + case DecodeHookFuncKind: + return func(from reflect.Value, to reflect.Value) (interface{}, error) { + return f(from.Kind(), to.Kind(), from.Interface()) + } + case DecodeHookFuncValue: + return func(from reflect.Value, to reflect.Value) (interface{}, error) { + return f(from, to) + } + default: + return func(from reflect.Value, to reflect.Value) (interface{}, error) { + return nil, errors.New("invalid decode hook signature") + } + } +} + // DecodeHookExec executes the given decode hook. This should be used // since it'll naturally degrade to the older backwards compatible DecodeHookFunc // that took reflect.Kind instead of reflect.Type. @@ -60,13 +84,17 @@ func DecodeHookExec( // The composed funcs are called in order, with the result of the // previous transformation. func ComposeDecodeHookFunc(fs ...DecodeHookFunc) DecodeHookFunc { + cached := make([]func(from reflect.Value, to reflect.Value) (interface{}, error), 0, len(fs)) + for _, f := range fs { + cached = append(cached, cachedDecodeHook(f)) + } return func(f reflect.Value, t reflect.Value) (interface{}, error) { var err error data := f.Interface() newFrom := f - for _, f1 := range fs { - data, err = DecodeHookExec(f1, newFrom, t) + for _, c := range cached { + data, err = c(newFrom, t) if err != nil { return nil, err } @@ -80,13 +108,17 @@ func ComposeDecodeHookFunc(fs ...DecodeHookFunc) DecodeHookFunc { // OrComposeDecodeHookFunc executes all input hook functions until one of them returns no error. In that case its value is returned. // If all hooks return an error, OrComposeDecodeHookFunc returns an error concatenating all error messages. func OrComposeDecodeHookFunc(ff ...DecodeHookFunc) DecodeHookFunc { + cached := make([]func(from reflect.Value, to reflect.Value) (interface{}, error), 0, len(ff)) + for _, f := range ff { + cached = append(cached, cachedDecodeHook(f)) + } return func(a, b reflect.Value) (interface{}, error) { var allErrs string var out interface{} var err error - for _, f := range ff { - out, err = DecodeHookExec(f, a, b) + for _, c := range cached { + out, err = c(a, b) if err != nil { allErrs += err.Error() + "\n" continue