Skip to content

incorrect assumption in mapstructure library causing panic #200

@jxsl13

Description

@jxsl13

mapstructure package assumes that when reflect.Type.Kind() == reflect.String is true that then interface{}.(string) will always succeed.

Below two tests, the first one panics due to the above assumption, the second one with a fix attempt that does not panic but still needs some improvements from someone who knows more about reflections.

The problem is that we pass a value type to koanf but the pointer type implements the TextMarshaler interface which we want to use before falling back to simply setting the internal string value.

The TODO show where I'm currently struggling...

package main_test

import (
	"encoding"
	"fmt"
	"reflect"
	"strings"
	"testing"

	"github.com/knadh/koanf"
	"github.com/knadh/koanf/providers/structs"
	"github.com/mitchellh/mapstructure"
	"github.com/stretchr/testify/assert"
)

type LogFormat string

func (c *LogFormat) UnmarshalText(data []byte) error {
       //overcomplicated custom internal string representation
        // in order to have a different internal representation from an external string representation
	switch strings.ToLower(string(data)) {
	case "", "json":
		*c = "json_custom"
	case "text":
		*c = "text_custom"
	default:
		return fmt.Errorf("invalid log format: %s", string(data))
	}
	return nil
}

func (c *LogFormat) MarshalText() ([]byte, error) {
	//overcomplicated custom internal string representation
        // in order to have a different internal representation from an external string representation
	switch *c {
	case "", "json_custom":
		return []byte("json"), nil
	case "text_custom":
		return []byte("text"), nil
	}
	return nil, fmt.Errorf("invalid internal string representation: %q", *c)
}

func TestTextUnmarshalStringBroken(t *testing.T) {
	defer func() {
		assert.Nil(t, recover())
	}()

	type targetStruct struct {
		LogFormat LogFormat // default should map to json
	}

	target := &targetStruct{"text_custom"}
	before := target.LogFormat

	k := koanf.New(".")
	k.Load(structs.Provider(target, "koanf"), nil)

	// default values explicitly set at top level in order to see the difference
	err := k.UnmarshalWithConf("",
		&target,
		koanf.UnmarshalConf{
			FlatPaths: true,
			DecoderConfig: &mapstructure.DecoderConfig{
				DecodeHook: mapstructure.ComposeDecodeHookFunc(
					mapstructure.StringToTimeDurationHookFunc(),
					mapstructure.StringToSliceHookFunc(","),
					mapstructure.TextUnmarshallerHookFunc()),
				Metadata:         nil,
				Result:           &target,
				WeaklyTypedInput: true,
			}})
	assert.NoError(t, err)
	assert.Equal(t, before, target.LogFormat)
}

func TestTextUnmarshalStringFixed(t *testing.T) {
	defer func() {
		assert.Nil(t, recover())
	}()

	type targetStruct struct {
		LogFormat LogFormat
	}

	target := &targetStruct{"text_custom"}
	before := target.LogFormat

	var b any = before

	_, ok := (b).(encoding.TextMarshaler)
	assert.True(t, ok)

	k := koanf.New(".")
	k.Load(structs.Provider(target, "koanf"), nil)

	// default values with a custom TextUnmarshalerHookFunc implementation
	err := k.UnmarshalWithConf("",
		&target,
		koanf.UnmarshalConf{
			FlatPaths: true,
			DecoderConfig: &mapstructure.DecoderConfig{
				DecodeHook: mapstructure.ComposeDecodeHookFunc(
					mapstructure.StringToTimeDurationHookFunc(),
					mapstructure.StringToSliceHookFunc(","),
					CustomTextUnmarshalHookFunc()), // our custom implementation
				Metadata:         nil,
				Result:           &target,
				WeaklyTypedInput: true,
			}})
	assert.NoError(t, err)
	assert.Equal(t, before, target.LogFormat)
}

func CustomTextUnmarshalHookFunc() mapstructure.DecodeHookFuncType {
	return func(
		f reflect.Type,
		t reflect.Type,
		data interface{}) (interface{}, error) {
		if f.Kind() != reflect.String {
			return data, nil
		}
		result := reflect.New(t).Interface()
		unmarshaller, ok := result.(encoding.TextUnmarshaler)
		if !ok {
			return data, nil
		}

		// default text representaion is the actual value of the `from` string
		var (
			dataVal = reflect.ValueOf(data)
			text    = []byte(dataVal.String())
		)
		if f.Kind() == t.Kind() {
			// source and target are of underlying type string
			var err error
			ptrVal := reflect.New(reflect.PointerTo(dataVal.Type()))
			ptrVal.SetPointer(dataVal.UnsafePointer())

			// TODO: we need to assert that both, the value type and the pointer type
			// do (not) implement the TextMarshaler interface before proceeding and simmply
			// using the the string value of the string type.
			// it might be the case that the internal string representation differs from
			// the (un)marshalled string.

			for _, v := range []reflect.Value{dataVal, ptrVal} {
				if marshaller, ok := v.Interface().(encoding.TextMarshaler); ok {
					text, err = marshaller.MarshalText()
					if err != nil {
						return nil, err
					}
				}
			}
		}

		if err := unmarshaller.UnmarshalText(text); err != nil {
			return nil, err
		}
		return result, nil
	}
}

func TestReflectPointerValueFromValueValue(t *testing.T) {
	defer func() {
		assert.Nil(t, recover())
	}()

	before := LogFormat("text_custom")
	var (
		b      any = before
		bptr   any = &before
		target     = reflect.ValueOf(bptr)
	)

	_, ok := (b).(encoding.TextMarshaler)
	if ok {
		panic("value type should not implement unmarshaler")
	}

	_, ok = (bptr).(encoding.TextMarshaler)
	if !ok {
		panic("pointer type should implement unmarshaler")
	}

	bVal := reflect.ValueOf(b)

	bptrVal := reflect.New(reflect.PointerTo(bVal.Type()))
	// TODO: do something so that the pointer value of bptrVal becomes the pointer to
	// b which is equivalent to the pointer inside of target



	if !target.Equal(bptrVal) {
		panic("btprValue is not the same as the target pointer value")
	}

	fmt.Println("successfully converted value type to pointer type")
}
```
 

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions