driver.Valuer wrong type causes stack overflow

[mcdoker18]

If driver.Valuer returns a type which is not listed in the Append function then it causes stack overflow.

Expected behaviour: the Append function returns an error.

Test:

type anotherString string

var _ driver.Valuer = (*anotherString)(nil)

func (v *anotherString) Scan(src interface{}) error {
	switch src := src.(type) {
	case []byte:
		*v = anotherString(src)
	case string:
		*v = anotherString(src)
	default:
		panic("not reached")
	}
	return nil
}

func (v anotherString) Value() (driver.Value, error) {
	return v, nil
}

func testWrongValuerTypeValuer(t *testing.T, db *bun.DB) {
	var expectedValue = anotherString("example value")

	type Model struct {
		ID    int           `bun:",pk,autoincrement"`
		Value anotherString `bun:"value"`
	}

	ctx := context.Background()

	err := db.ResetModel(ctx, (*Model)(nil))
	require.NoError(t, err)

	model := &Model{Value: expectedValue}
	_, err = db.NewInsert().Model(model).Exec(ctx)
	require.NoError(t, err)

	model2 := new(Model)
	err = db.NewSelect().Model(model2).Scan(ctx)
	require.NoError(t, err)
	require.Equal(t, expectedValue, model2.Value)
}

Stacktrace:

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc020b823c8 stack=[0xc020b82000, 0xc040b82000]
fatal error: stack overflow

runtime stack:
runtime.throw({0x195fd4b?, 0x2096b60?})
	/usr/local/opt/go/libexec/src/runtime/panic.go:1047 +0x5d fp=0x70000e943d80 sp=0x70000e943d50 pc=0x10389dd
runtime.newstack()
	/usr/local/opt/go/libexec/src/runtime/stack.go:1103 +0x5cc fp=0x70000e943f38 sp=0x70000e943d80 pc=0x1052d4c
runtime.morestack()
	/usr/local/opt/go/libexec/src/runtime/asm_amd64.s:570 +0x8b fp=0x70000e943f40 sp=0x70000e943f38 pc=0x106ac0b

goroutine 345 [running]:
runtime.heapBitsSetType(0xc001eb68e0?, 0x10?, 0x10?, 0x186eea0?)
	/usr/local/opt/go/libexec/src/runtime/mbitmap.go:844 +0xbac fp=0xc020b823d8 sp=0xc020b823d0 pc=0x1017aec
runtime.mallocgc(0x10, 0x186eea0, 0x1)
	/usr/local/opt/go/libexec/src/runtime/malloc.go:1050 +0x64d fp=0xc020b82450 sp=0xc020b823d8 pc=0x100f1ed
runtime.convTstring({0x195c1c5, 0xd})
	/usr/local/opt/go/libexec/src/runtime/iface.go:392 +0x47 fp=0xc020b82478 sp=0xc020b82450 pc=0x100cd67
github.com/uptrace/bun/internal/dbtest_test.anotherString.Value(...)
	/Users/vitaliisolodilov/opensource/uptrace/bun/internal/dbtest/db_test.go:832
github.com/uptrace/bun/internal/dbtest_test.(*anotherString).Value(0x1889480?)
	<autogenerated>:1 +0x34 fp=0xc020b82498 sp=0xc020b82478 pc=0x1826574
github.com/uptrace/bun/schema.appendDriverValue({{0x1ac46c8?, 0xc00010f9b0?}, 0xc000380f60?}, {0xc000675000, 0x36, 0x1000}, {0x1889480?, 0xc001eb68d0?, 0xc000580480?})
	/Users/vitaliisolodilov/opensource/uptrace/bun/schema/append_value.go:273 +0xb3 fp=0xc020b82550 sp=0xc020b82498 pc=0x13dbdf3
github.com/uptrace/bun/schema.Append({{0x1ac46c8?, 0xc00010f9b0?}, 0xc000380f60?}, {0xc000675000, 0x36, 0x1000}, {0x1889480?, 0xc001eb68d0})
	/Users/vitaliisolodilov/opensource/uptrace/bun/schema/append.go:48 +0x6b5 fp=0xc020b82608 sp=0xc020b82550 pc=0x13d8cd5

[vmihailenco]

The problem here is that Value() returns a value that implements driver.Valuer interface so Bun tries to unwrap that too.

We could add a check to appendDriverValue to stop recursion:

func appendDriverValue(fmter Formatter, b []byte, v reflect.Value) []byte {
	value, err := v.Interface().(driver.Valuer).Value()
	if err != nil {
		return dialect.AppendError(b, err)
	}
	if _, ok := value.(driver.Valuer); ok {
		return dialect.AppendError(b, fmt.Errorf("...."))
	}
	return Append(fmter, b, value)
}

Could you test that suggestion?

[mcdoker18]

The problem here is that Value() returns a value that implements driver.Valuer interface so Bun tries to unwrap that too.

We could add a check to appendDriverValue to stop recursion:

func appendDriverValue(fmter Formatter, b []byte, v reflect.Value) []byte {
	value, err := v.Interface().(driver.Valuer).Value()
	if err != nil {
		return dialect.AppendError(b, err)
	}
	if _, ok := value.(driver.Valuer); ok {
		return dialect.AppendError(b, fmt.Errorf("...."))
	}
	return Append(fmter, b, value)
}

Could you test that suggestion?

Thanks for the suggestion, it works!