Fix GH-340: Decoding array of slices causes panic

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

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)
 		}
@@ -1163,7 +1167,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 {

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)
+	}
+}