1.4.0: decode hooks receive the wrong type for the to argument

[dnephin]

I attempted to upgrade from mapstructure 1.3.3 to 1.4.0 in Consul and a few of the test failed.

Debugging one of the tests it seems to be that the decode hook (reflect.Type hook) is receiving map[string]interface {} as the type of the to argument, where as previously it received time.Time as the type of to.

Test failures can be seen here: https://app.circleci.com/pipelines/github/hashicorp/consul/15588/workflows/76c7848b-f780-4049-aa5c-f091b737207b/jobs/302146

I suspect the change which added a new reflect.Value version of hooks may have broken this. I'll keep looking to see if I can find a fix.

More details:

1. squash is not being used
2. the decode is struct->struct
3. this particular hook is checking the type of both from and to
4. previously the decode hook would be called twice, now the decode hook is being called 4 times
5. previously it was the second call which receives a from and to of the correct types, now the second call receives the right type for the from, and the forth call receives the correct type for to. In both cases the other type is map[string]interface {}, even though neither the input or output contain a map.

I suspect it is not actually the change to hooks that broke this. It seems like it was one of the changes to decodeStructFromMap.

[dnephin]

Reverting 6023400 (#205) restores the old behaviour, so that seems to be the source of the regression.

[mitchellh]

I'm trying to write a minimal test case just to ensure we have a regression test for this and I'm currently struggling to get the right behavior. Right now I have this (which passes, so its NOT showing the bug yet):

func TestDecode_DecodeHookType_time(t *testing.T) {
	t.Parallel()

	input := struct {
		Time int64
	}{
		Time: time.Now().Unix(),
	}

	expectedType := reflect.TypeOf(time.Now())

	decodeHook := func(from reflect.Type, to reflect.Type, v interface{}) (interface{}, error) {
		if from.Kind() == reflect.Int64 &&
			to == expectedType {
			return time.Now(), nil
		}

		return v, nil
	}

	var result struct {
		Time time.Time
	}
	config := &DecoderConfig{
		DecodeHook: decodeHook,
		Result:     &result,
	}

	decoder, err := NewDecoder(config)
	if err != nil {
		t.Fatalf("err: %s", err)
	}

	err = decoder.Decode(input)
	if err != nil {
		t.Fatalf("got an err: %s", err)
	}

	if result.Time.IsZero() {
		t.Errorf("vint should not be 0: %#v", result.Time)
	}
}

[mitchellh]

I got a PR up to fix it but no new test added to prevent this, although that test change would've caught it :)

[mitchellh]

Found a test in #228 verified

[mitchellh]

Okay the test in #228 failed without my patch (based on your findings) and passes with the patch. I feel very confident that fixes your issue. If you can verify and let me know I'll merge and cut a patch release.