Skip to content

Commit a4b80e8

Browse files
authored
fix(contrib/drivers/pgsql): preserve bytea data integrity on read and write (gogf#4678)
## Summary Fix two bytea data corruption issues in the PostgreSQL driver: 1. **READ path** (fixes gogf#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 gogf#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 gogf#4677 closes gogf#4231 ref gogf#4689
1 parent 0e1cb15 commit a4b80e8

3 files changed

Lines changed: 132 additions & 1 deletion

File tree

contrib/drivers/pgsql/pgsql_convert.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ func (d *Driver) ConvertValueForField(ctx context.Context, fieldType string, fie
3030
var fieldValueKind = reflect.TypeOf(fieldValue).Kind()
3131

3232
if fieldValueKind == reflect.Slice {
33+
// For bytea type, pass []byte directly without any conversion.
34+
if _, ok := fieldValue.([]byte); ok && gstr.Contains(fieldType, "bytea") {
35+
return d.Core.ConvertValueForField(ctx, fieldType, fieldValue)
36+
}
3337
// For pgsql, json or jsonb require '[]'
3438
if !gstr.Contains(fieldType, "json") {
3539
fieldValue = gstr.ReplaceByMap(gconv.String(fieldValue),
@@ -62,6 +66,7 @@ func (d *Driver) ConvertValueForField(ctx context.Context, fieldType string, fie
6266
// | _varchar, _text | []string |
6367
// | _char, _bpchar | []string |
6468
// | _numeric, _decimal, _money | []float64 |
69+
// | bytea | []byte |
6570
// | _bytea | [][]byte |
6671
// | _uuid | []uuid.UUID |
6772
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
107112
case "_numeric", "_decimal", "_money":
108113
return gdb.LocalTypeFloat64Slice, nil
109114

115+
case "bytea":
116+
return gdb.LocalTypeBytes, nil
117+
110118
case "_bytea":
111119
return gdb.LocalTypeBytesSlice, nil
112120

@@ -141,6 +149,7 @@ func (d *Driver) CheckLocalTypeForField(ctx context.Context, fieldType string, f
141149
// | _numeric | numeric[] | pq.Float64Array | []float64 |
142150
// | _decimal | decimal[] | pq.Float64Array | []float64 |
143151
// | _money | money[] | pq.Float64Array | []float64 |
152+
// | bytea | bytea | - | []byte |
144153
// | _bytea | bytea[] | pq.ByteaArray | [][]byte |
145154
// | _uuid | uuid[] | pq.StringArray | []uuid.UUID |
146155
//
@@ -151,9 +160,16 @@ func (d *Driver) ConvertValueForLocal(ctx context.Context, fieldType string, fie
151160
typeName, _ := gregex.ReplaceString(`\(.+\)`, "", fieldType)
152161
typeName = strings.ToLower(typeName)
153162

154-
// Basic types are mostly handled by Core layer, only handle array types here
163+
// Basic types are mostly handled by Core layer; handle array types and special-case bytea here.
155164
switch typeName {
156165

166+
// []byte
167+
case "bytea":
168+
if v, ok := fieldValue.([]byte); ok {
169+
return v, nil
170+
}
171+
return fieldValue, nil
172+
157173
// []int32
158174
case "_int2", "_int4":
159175
var result pq.Int32Array

contrib/drivers/pgsql/pgsql_z_unit_convert_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,13 @@ func Test_CheckLocalTypeForField(t *testing.T) {
108108
t.Assert(localType, gdb.LocalTypeFloat64Slice)
109109
})
110110

111+
gtest.C(t, func(t *gtest.T) {
112+
// Test bytea type
113+
localType, err := driver.CheckLocalTypeForField(ctx, "bytea", nil)
114+
t.AssertNil(err)
115+
t.Assert(localType, gdb.LocalTypeBytes)
116+
})
117+
111118
gtest.C(t, func(t *gtest.T) {
112119
// Test bytea array type
113120
localType, err := driver.CheckLocalTypeForField(ctx, "_bytea", nil)
@@ -362,6 +369,17 @@ func Test_ConvertValueForLocal(t *testing.T) {
362369
_, err := driver.ConvertValueForLocal(ctx, "_bytea", "invalid")
363370
t.AssertNE(err, nil)
364371
})
372+
373+
gtest.C(t, func(t *gtest.T) {
374+
// Test bytea conversion - should preserve []byte as-is
375+
input := []byte{0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x5D, 0x5B}
376+
result, err := driver.ConvertValueForLocal(ctx, "bytea", input)
377+
t.AssertNil(err)
378+
resultBytes, ok := result.([]byte)
379+
t.Assert(ok, true)
380+
t.Assert(len(resultBytes), len(input))
381+
t.Assert(resultBytes, input)
382+
})
365383
}
366384

367385
// Test_ConvertValueForField tests the ConvertValueForField method
@@ -406,4 +424,14 @@ func Test_ConvertValueForField(t *testing.T) {
406424
t.AssertNil(err)
407425
t.Assert(result, `["a","b"]`)
408426
})
427+
428+
gtest.C(t, func(t *gtest.T) {
429+
// Test []byte value for bytea type (should preserve raw bytes, not do []->{} replacement)
430+
input := []byte{0xDE, 0xAD, 0x5B, 0x5D, 0xBE, 0xEF}
431+
result, err := driver.ConvertValueForField(ctx, "bytea", input)
432+
t.AssertNil(err)
433+
resultBytes, ok := result.([]byte)
434+
t.Assert(ok, true)
435+
t.Assert(resultBytes, input)
436+
})
409437
}

contrib/drivers/pgsql/pgsql_z_unit_issue_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,93 @@ func Test_Issue4500(t *testing.T) {
290290
})
291291
}
292292

293+
// https://github.com/gogf/gf/issues/4677
294+
// record.Get().Bytes() corrupts bytea data on retrieval from PostgreSQL.
295+
func Test_Issue4677(t *testing.T) {
296+
table := fmt.Sprintf(`%s_%d`, TablePrefix+"issue4677", gtime.TimestampNano())
297+
if _, err := db.Exec(ctx, fmt.Sprintf(`
298+
CREATE TABLE %s (
299+
id bigserial PRIMARY KEY,
300+
bin_data bytea
301+
);`, table,
302+
)); err != nil {
303+
gtest.Fatal(err)
304+
}
305+
defer dropTable(table)
306+
307+
gtest.C(t, func(t *gtest.T) {
308+
// Test 1: Binary data with various byte values including 0x00, 0x5D(']'), 0x5B('[')
309+
originalBytes := []byte{
310+
0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x01, 0x5B, 0x5D,
311+
0xFF, 0x7B, 0x7D, 0x80, 0xCA, 0xFE, 0xBA, 0xBE,
312+
}
313+
314+
_, err := db.Model(table).Data(g.Map{
315+
"bin_data": originalBytes,
316+
}).Insert()
317+
t.AssertNil(err)
318+
319+
record, err := db.Model(table).Where("id", 1).One()
320+
t.AssertNil(err)
321+
322+
retrievedBytes := record["bin_data"].Bytes()
323+
t.Assert(len(retrievedBytes), len(originalBytes))
324+
t.Assert(retrievedBytes, originalBytes)
325+
})
326+
327+
gtest.C(t, func(t *gtest.T) {
328+
// Test 2: Larger binary data (simulating gob/protobuf encoded payload)
329+
largeBytes := make([]byte, 1024)
330+
for i := range largeBytes {
331+
largeBytes[i] = byte(i % 256)
332+
}
333+
334+
_, err := db.Model(table).Data(g.Map{
335+
"bin_data": largeBytes,
336+
}).Insert()
337+
t.AssertNil(err)
338+
339+
record, err := db.Model(table).OrderDesc("id").One()
340+
t.AssertNil(err)
341+
342+
retrievedBytes := record["bin_data"].Bytes()
343+
t.Assert(len(retrievedBytes), len(largeBytes))
344+
t.Assert(retrievedBytes, largeBytes)
345+
})
346+
}
347+
348+
// https://github.com/gogf/gf/issues/4231
349+
// ConvertValueForField corrupts bytea data containing 0x5D on write.
350+
func Test_Issue4231(t *testing.T) {
351+
table := fmt.Sprintf(`%s_%d`, TablePrefix+"issue4231", gtime.TimestampNano())
352+
if _, err := db.Exec(ctx, fmt.Sprintf(`
353+
CREATE TABLE %s (
354+
id bigserial PRIMARY KEY,
355+
bin_data bytea
356+
);`, table,
357+
)); err != nil {
358+
gtest.Fatal(err)
359+
}
360+
defer dropTable(table)
361+
362+
gtest.C(t, func(t *gtest.T) {
363+
// Bytes containing 0x5D (ASCII ']') which was being converted to 0x7D ('}')
364+
originalBytes := []byte{0x01, 0x5D, 0x02, 0x5B, 0x03}
365+
366+
_, err := db.Model(table).Data(g.Map{
367+
"bin_data": originalBytes,
368+
}).Insert()
369+
t.AssertNil(err)
370+
371+
record, err := db.Model(table).Where("id", 1).One()
372+
t.AssertNil(err)
373+
374+
retrievedBytes := record["bin_data"].Bytes()
375+
t.Assert(len(retrievedBytes), len(originalBytes))
376+
t.Assert(retrievedBytes, originalBytes)
377+
})
378+
}
379+
293380
// https://github.com/gogf/gf/issues/4595
294381
// FieldsPrefix silently drops fields when using table alias before LeftJoin.
295382
func Test_Issue4595(t *testing.T) {

0 commit comments

Comments
 (0)