From 38e09bda4c68fc87967b50455640e308bda6a959 Mon Sep 17 00:00:00 2001 From: Jack Christensen Date: Tue, 21 Feb 2023 20:55:57 -0600 Subject: [PATCH] Fix *wrapSliceEncodePlan[T].Encode 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. https://github.com/jackc/pgx/issues/1502 --- pgtype/pgtype.go | 6 +----- pgtype/pgtype_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/pgtype/pgtype.go b/pgtype/pgtype.go index 3bddf2ae..83b349ce 100644 --- a/pgtype/pgtype.go +++ b/pgtype/pgtype.go @@ -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 { diff --git a/pgtype/pgtype_test.go b/pgtype/pgtype_test.go index 60863f49..a3870d51 100644 --- a/pgtype/pgtype_test.go +++ b/pgtype/pgtype_test.go @@ -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}