Skip to content

Commit

Permalink
Fix *wrapSliceEncodePlan[T].Encode
Browse files Browse the repository at this point in the history
It should pass a FlatArray[T] to the next step instead of a
anySliceArrayReflect. By using a anySliceArrayReflect, an encode of
[]github.com/google/uuid.UUID followed by []string into a PostgreSQL
uuid[] would crash. This was caused by a EncodePlan cache collision
where the second encoding used part of the cached plan of the first.

In proper usage a cache collision shouldn't be able to occur. If this
assertion proves incorrect it will be necessary to add an optional
interface to ScanPlan and EncodePlan that marks the plan as ineligable
for caching. But I have been unable to construct a failing case, and
given that ScanPlans have been cached for quite some time now without
incident I do not think it is possible. This issue only occurred due to
the bug in *wrapSliceEncodePlan[T].Encode.

#1502
  • Loading branch information
jackc committed Feb 22, 2023
1 parent 9567297 commit 38e09bd
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
6 changes: 1 addition & 5 deletions pgtype/pgtype.go
Original file line number Diff line number Diff line change
Expand Up @@ -1945,11 +1945,7 @@ type wrapSliceEncodePlan[T any] struct {
func (plan *wrapSliceEncodePlan[T]) SetNext(next EncodePlan) { plan.next = next }

func (plan *wrapSliceEncodePlan[T]) Encode(value any, buf []byte) (newBuf []byte, err error) {
w := anySliceArrayReflect{
slice: reflect.ValueOf(value),
}

return plan.next.Encode(w, buf)
return plan.next.Encode((FlatArray[T])(value.([]T)), buf)
}

type wrapSliceEncodeReflectPlan struct {
Expand Down
30 changes: 30 additions & 0 deletions pgtype/pgtype_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,36 @@ func TestMapScanNullToWrongType(t *testing.T) {
assert.False(t, pn.Valid)
}

type databaseValuerUUID [16]byte

func (v databaseValuerUUID) Value() (driver.Value, error) {
return fmt.Sprintf("%x", v), nil
}

// https://github.com/jackc/pgx/issues/1502
func TestMapEncodePlanCacheUUIDTypeConfusion(t *testing.T) {
expected := []byte{
0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0xb, 0x86, 0, 0, 0, 2, 0, 0, 0, 1,
0, 0, 0, 16,
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
0, 0, 0, 16,
15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0}

m := pgtype.NewMap()
buf, err := m.Encode(pgtype.UUIDArrayOID, pgtype.BinaryFormatCode,
[]databaseValuerUUID{{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}, {15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}},
nil)
require.NoError(t, err)
require.Equal(t, expected, buf)

// This actually *should* fail. In the actual query path this error is detected and the encoding falls back to the
// text format. In the bug this test is guarding against regression this would panic.
_, err = m.Encode(pgtype.UUIDArrayOID, pgtype.BinaryFormatCode,
[]string{"00010203-0405-0607-0809-0a0b0c0d0e0f", "0f0e0d0c-0b0a-0908-0706-0504-03020100"},
nil)
require.Error(t, err)
}

func BenchmarkMapScanInt4IntoBinaryDecoder(b *testing.B) {
m := pgtype.NewMap()
src := []byte{0, 0, 0, 42}
Expand Down

0 comments on commit 38e09bd

Please sign in to comment.