From a4b80e8680aaa45aa6d42272a9b4980a76329c80 Mon Sep 17 00:00:00 2001 From: Jack Ling <34231795+lingcoder@users.noreply.github.com> Date: Fri, 27 Feb 2026 16:22:43 +0800 Subject: [PATCH] fix(contrib/drivers/pgsql): preserve bytea data integrity on read and write (#4678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fix two bytea data corruption issues in the PostgreSQL driver: 1. **READ path** (fixes #4677): `CheckLocalTypeForField` and `ConvertValueForLocal` had no case for plain `bytea` type, causing it to fall through to the Core layer which incorrectly mapped it to `LocalTypeString`. Binary data was then converted to string via `gconv.String()`, corrupting the bytes on retrieval. 2. **WRITE path** (fixes #4231): `ConvertValueForField` applied PostgreSQL array syntax conversion (`[` → `{`, `]` → `}`) to all slice types including `[]byte` for bytea columns, corrupting bytes `0x5B` (`[`) and `0x5D` (`]`) on insertion. ## Changes - **`contrib/drivers/pgsql/pgsql_convert.go`**: - `CheckLocalTypeForField`: Add `case "bytea"` → `LocalTypeBytes` - `ConvertValueForLocal`: Add `case "bytea"` to preserve `[]byte` as-is - `ConvertValueForField`: Skip `[]`→`{}` replacement for `[]byte` with `bytea` field type - **`contrib/drivers/pgsql/pgsql_z_unit_convert_test.go`**: - Add unit tests for `bytea` type in `CheckLocalTypeForField`, `ConvertValueForLocal`, and `ConvertValueForField` - **`contrib/drivers/pgsql/pgsql_z_unit_issue_test.go`**: - Add `Test_Issue4677`: End-to-end round-trip test with various binary data (including 0x00, 0x5B, 0x5D, 0xFF) - Add `Test_Issue4231`: Targeted test for 0x5D byte corruption on write ## Test plan - [x] `Test_CheckLocalTypeForField` - bytea returns `LocalTypeBytes` - [x] `Test_ConvertValueForLocal` - bytea preserves `[]byte` as-is - [x] `Test_ConvertValueForField` - bytea skips array syntax replacement - [x] `Test_Issue4677` - full DB round-trip with binary data - [x] `Test_Issue4231` - write path preserves 0x5B/0x5D bytes - [x] Full pgsql test suite passes with no regressions closes #4677 closes #4231 ref #4689 --- contrib/drivers/pgsql/pgsql_convert.go | 18 +++- .../pgsql/pgsql_z_unit_convert_test.go | 28 ++++++ .../drivers/pgsql/pgsql_z_unit_issue_test.go | 87 +++++++++++++++++++ 3 files changed, 132 insertions(+), 1 deletion(-) diff --git a/contrib/drivers/pgsql/pgsql_convert.go b/contrib/drivers/pgsql/pgsql_convert.go index 55c310b65..e5dd1f014 100644 --- a/contrib/drivers/pgsql/pgsql_convert.go +++ b/contrib/drivers/pgsql/pgsql_convert.go @@ -30,6 +30,10 @@ func (d *Driver) ConvertValueForField(ctx context.Context, fieldType string, fie var fieldValueKind = reflect.TypeOf(fieldValue).Kind() if fieldValueKind == reflect.Slice { + // For bytea type, pass []byte directly without any conversion. + if _, ok := fieldValue.([]byte); ok && gstr.Contains(fieldType, "bytea") { + return d.Core.ConvertValueForField(ctx, fieldType, fieldValue) + } // For pgsql, json or jsonb require '[]' if !gstr.Contains(fieldType, "json") { fieldValue = gstr.ReplaceByMap(gconv.String(fieldValue), @@ -62,6 +66,7 @@ func (d *Driver) ConvertValueForField(ctx context.Context, fieldType string, fie // | _varchar, _text | []string | // | _char, _bpchar | []string | // | _numeric, _decimal, _money | []float64 | +// | bytea | []byte | // | _bytea | [][]byte | // | _uuid | []uuid.UUID | func (d *Driver) CheckLocalTypeForField(ctx context.Context, fieldType string, fieldValue any) (gdb.LocalType, error) { @@ -107,6 +112,9 @@ func (d *Driver) CheckLocalTypeForField(ctx context.Context, fieldType string, f case "_numeric", "_decimal", "_money": return gdb.LocalTypeFloat64Slice, nil + case "bytea": + return gdb.LocalTypeBytes, nil + case "_bytea": return gdb.LocalTypeBytesSlice, nil @@ -141,6 +149,7 @@ func (d *Driver) CheckLocalTypeForField(ctx context.Context, fieldType string, f // | _numeric | numeric[] | pq.Float64Array | []float64 | // | _decimal | decimal[] | pq.Float64Array | []float64 | // | _money | money[] | pq.Float64Array | []float64 | +// | bytea | bytea | - | []byte | // | _bytea | bytea[] | pq.ByteaArray | [][]byte | // | _uuid | uuid[] | pq.StringArray | []uuid.UUID | // @@ -151,9 +160,16 @@ func (d *Driver) ConvertValueForLocal(ctx context.Context, fieldType string, fie typeName, _ := gregex.ReplaceString(`\(.+\)`, "", fieldType) typeName = strings.ToLower(typeName) - // Basic types are mostly handled by Core layer, only handle array types here + // Basic types are mostly handled by Core layer; handle array types and special-case bytea here. switch typeName { + // []byte + case "bytea": + if v, ok := fieldValue.([]byte); ok { + return v, nil + } + return fieldValue, nil + // []int32 case "_int2", "_int4": var result pq.Int32Array diff --git a/contrib/drivers/pgsql/pgsql_z_unit_convert_test.go b/contrib/drivers/pgsql/pgsql_z_unit_convert_test.go index 62bf92e6e..2936659b0 100644 --- a/contrib/drivers/pgsql/pgsql_z_unit_convert_test.go +++ b/contrib/drivers/pgsql/pgsql_z_unit_convert_test.go @@ -108,6 +108,13 @@ func Test_CheckLocalTypeForField(t *testing.T) { t.Assert(localType, gdb.LocalTypeFloat64Slice) }) + gtest.C(t, func(t *gtest.T) { + // Test bytea type + localType, err := driver.CheckLocalTypeForField(ctx, "bytea", nil) + t.AssertNil(err) + t.Assert(localType, gdb.LocalTypeBytes) + }) + gtest.C(t, func(t *gtest.T) { // Test bytea array type localType, err := driver.CheckLocalTypeForField(ctx, "_bytea", nil) @@ -362,6 +369,17 @@ func Test_ConvertValueForLocal(t *testing.T) { _, err := driver.ConvertValueForLocal(ctx, "_bytea", "invalid") t.AssertNE(err, nil) }) + + gtest.C(t, func(t *gtest.T) { + // Test bytea conversion - should preserve []byte as-is + input := []byte{0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x5D, 0x5B} + result, err := driver.ConvertValueForLocal(ctx, "bytea", input) + t.AssertNil(err) + resultBytes, ok := result.([]byte) + t.Assert(ok, true) + t.Assert(len(resultBytes), len(input)) + t.Assert(resultBytes, input) + }) } // Test_ConvertValueForField tests the ConvertValueForField method @@ -406,4 +424,14 @@ func Test_ConvertValueForField(t *testing.T) { t.AssertNil(err) t.Assert(result, `["a","b"]`) }) + + gtest.C(t, func(t *gtest.T) { + // Test []byte value for bytea type (should preserve raw bytes, not do []->{} replacement) + input := []byte{0xDE, 0xAD, 0x5B, 0x5D, 0xBE, 0xEF} + result, err := driver.ConvertValueForField(ctx, "bytea", input) + t.AssertNil(err) + resultBytes, ok := result.([]byte) + t.Assert(ok, true) + t.Assert(resultBytes, input) + }) } diff --git a/contrib/drivers/pgsql/pgsql_z_unit_issue_test.go b/contrib/drivers/pgsql/pgsql_z_unit_issue_test.go index 0e706916d..95f953fe2 100644 --- a/contrib/drivers/pgsql/pgsql_z_unit_issue_test.go +++ b/contrib/drivers/pgsql/pgsql_z_unit_issue_test.go @@ -290,6 +290,93 @@ func Test_Issue4500(t *testing.T) { }) } +// https://github.com/gogf/gf/issues/4677 +// record.Get().Bytes() corrupts bytea data on retrieval from PostgreSQL. +func Test_Issue4677(t *testing.T) { + table := fmt.Sprintf(`%s_%d`, TablePrefix+"issue4677", gtime.TimestampNano()) + if _, err := db.Exec(ctx, fmt.Sprintf(` + CREATE TABLE %s ( + id bigserial PRIMARY KEY, + bin_data bytea + );`, table, + )); err != nil { + gtest.Fatal(err) + } + defer dropTable(table) + + gtest.C(t, func(t *gtest.T) { + // Test 1: Binary data with various byte values including 0x00, 0x5D(']'), 0x5B('[') + originalBytes := []byte{ + 0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01, 0x5B, 0x5D, + 0xFF, 0x7B, 0x7D, 0x80, 0xCA, 0xFE, 0xBA, 0xBE, + } + + _, err := db.Model(table).Data(g.Map{ + "bin_data": originalBytes, + }).Insert() + t.AssertNil(err) + + record, err := db.Model(table).Where("id", 1).One() + t.AssertNil(err) + + retrievedBytes := record["bin_data"].Bytes() + t.Assert(len(retrievedBytes), len(originalBytes)) + t.Assert(retrievedBytes, originalBytes) + }) + + gtest.C(t, func(t *gtest.T) { + // Test 2: Larger binary data (simulating gob/protobuf encoded payload) + largeBytes := make([]byte, 1024) + for i := range largeBytes { + largeBytes[i] = byte(i % 256) + } + + _, err := db.Model(table).Data(g.Map{ + "bin_data": largeBytes, + }).Insert() + t.AssertNil(err) + + record, err := db.Model(table).OrderDesc("id").One() + t.AssertNil(err) + + retrievedBytes := record["bin_data"].Bytes() + t.Assert(len(retrievedBytes), len(largeBytes)) + t.Assert(retrievedBytes, largeBytes) + }) +} + +// https://github.com/gogf/gf/issues/4231 +// ConvertValueForField corrupts bytea data containing 0x5D on write. +func Test_Issue4231(t *testing.T) { + table := fmt.Sprintf(`%s_%d`, TablePrefix+"issue4231", gtime.TimestampNano()) + if _, err := db.Exec(ctx, fmt.Sprintf(` + CREATE TABLE %s ( + id bigserial PRIMARY KEY, + bin_data bytea + );`, table, + )); err != nil { + gtest.Fatal(err) + } + defer dropTable(table) + + gtest.C(t, func(t *gtest.T) { + // Bytes containing 0x5D (ASCII ']') which was being converted to 0x7D ('}') + originalBytes := []byte{0x01, 0x5D, 0x02, 0x5B, 0x03} + + _, err := db.Model(table).Data(g.Map{ + "bin_data": originalBytes, + }).Insert() + t.AssertNil(err) + + record, err := db.Model(table).Where("id", 1).One() + t.AssertNil(err) + + retrievedBytes := record["bin_data"].Bytes() + t.Assert(len(retrievedBytes), len(originalBytes)) + t.Assert(retrievedBytes, originalBytes) + }) +} + // https://github.com/gogf/gf/issues/4595 // FieldsPrefix silently drops fields when using table alias before LeftJoin. func Test_Issue4595(t *testing.T) {