Skip to content

Commit b95ec0e

Browse files
vitess-bot[bot]tanjinx
vitess-bot[bot]
authored and
tanjinx
committed
[release-22.0] binlog_json: fix opaque value parsing to read variable-length (vitessio#19102) (vitessio#19109)
Signed-off-by: Nick Van Wiggeren <nick@planetscale.com> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Signed-off-by: Tanjin Xu <tanjin.xu@slack-corp.com>
1 parent 5eab7fa commit b95ec0e

File tree

2 files changed

+77
-18
lines changed

2 files changed

+77
-18
lines changed

‎go/mysql/binlog/binlog_json.go‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -449,11 +449,25 @@ func binparserLiteral(_ jsonDataType, data []byte, pos int) (node *json.Value, e
449449
// other types are stored as catch-all opaque types: documentation on these is scarce.
450450
// we currently know about (and support) date/time/datetime/decimal.
451451
func binparserOpaque(_ jsonDataType, data []byte, pos int) (node *json.Value, err error) {
452-
dataType := data[pos]
453-
start := 3 // account for length of stored value
454-
end := start + 8 // all currently supported opaque data types are 8 bytes in size
452+
if pos >= len(data) {
453+
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "opaque JSON field value missing type at position %d", pos)
454+
}
455+
typePos := pos
456+
dataType := data[typePos]
457+
pos = typePos + 1
458+
if pos >= len(data) {
459+
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "opaque JSON field value missing length at position %d", typePos)
460+
}
461+
length, start := readVariableLength(data, pos)
462+
end := start + length
463+
if start > len(data) || end > len(data) {
464+
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "opaque JSON field value length %d exceeds available bytes", length)
465+
}
455466
switch dataType {
456467
case TypeDate:
468+
if length < 8 {
469+
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "opaque date length %d is too short", length)
470+
}
457471
raw := binary.LittleEndian.Uint64(data[start:end])
458472
value := raw >> 24
459473
yearMonth := (value >> 22) & 0x01ffff // 17 bits starting at 22nd
@@ -463,6 +477,9 @@ func binparserOpaque(_ jsonDataType, data []byte, pos int) (node *json.Value, er
463477
dateString := fmt.Sprintf("%04d-%02d-%02d", year, month, day)
464478
node = json.NewDate(dateString)
465479
case TypeTime2, TypeTime:
480+
if length < 8 {
481+
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "opaque time length %d is too short", length)
482+
}
466483
raw := binary.LittleEndian.Uint64(data[start:end])
467484
value := raw >> 24
468485
hour := (value >> 12) & 0x03ff // 10 bits starting at 12th
@@ -472,6 +489,9 @@ func binparserOpaque(_ jsonDataType, data []byte, pos int) (node *json.Value, er
472489
timeString := fmt.Sprintf("%02d:%02d:%02d.%06d", hour, minute, second, microSeconds)
473490
node = json.NewTime(timeString)
474491
case TypeDateTime2, TypeDateTime, TypeTimestamp2, TypeTimestamp:
492+
if length < 8 {
493+
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "opaque datetime length %d is too short", length)
494+
}
475495
raw := binary.LittleEndian.Uint64(data[start:end])
476496
value := raw >> 24
477497
yearMonth := (value >> 22) & 0x01ffff // 17 bits starting at 22nd
@@ -485,6 +505,9 @@ func binparserOpaque(_ jsonDataType, data []byte, pos int) (node *json.Value, er
485505
timeString := fmt.Sprintf("%04d-%02d-%02d %02d:%02d:%02d.%06d", year, month, day, hour, minute, second, microSeconds)
486506
node = json.NewDateTime(timeString)
487507
case TypeDecimal, TypeNewDecimal:
508+
if length < 2 {
509+
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "opaque decimal length %d is too short", length)
510+
}
488511
decimalData := data[start:end]
489512
precision := decimalData[0]
490513
scale := decimalData[1]
@@ -495,11 +518,11 @@ func binparserOpaque(_ jsonDataType, data []byte, pos int) (node *json.Value, er
495518
}
496519
node = json.NewNumber(val.ToString(), json.NumberTypeDecimal)
497520
case TypeVarchar, TypeVarString, TypeString, TypeBlob, TypeTinyBlob, TypeMediumBlob, TypeLongBlob:
498-
node = json.NewBlob(string(data[pos+1:]))
521+
node = json.NewBlob(string(data[start:end]))
499522
case TypeBit:
500-
node = json.NewBit(string(data[pos+1:]))
523+
node = json.NewBit(string(data[start:end]))
501524
default:
502-
node = json.NewOpaqueValue(string(data[pos+1:]))
525+
node = json.NewOpaqueValue(string(data[start:end]))
503526
}
504527
return node, nil
505528
}

‎go/mysql/binlog/binlog_json_test.go‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -232,19 +232,24 @@ func TestBinaryJSON(t *testing.T) {
232232
expected: json.NewNumber("123456789.1234", json.NumberTypeDecimal),
233233
},
234234
{
235-
name: `bit literal [2 202 254]`,
235+
name: `small decimal "1.99"`,
236+
data: []byte{15, 246, 4, 3, 2, 0x81, 0x63},
237+
expected: json.NewNumber("1.99", json.NumberTypeDecimal),
238+
},
239+
{
240+
name: `bit literal 0xCAFE`,
236241
data: []byte{15, 16, 2, 202, 254},
237-
expected: json.NewBit(string([]byte{2, 202, 254})),
242+
expected: json.NewBit(string([]byte{202, 254})),
238243
},
239244
{
240-
name: `opaque string [2 202 254]`,
245+
name: `opaque string 0xCAFE`,
241246
data: []byte{15, 15, 2, 202, 254},
242-
expected: json.NewBlob(string([]byte{2, 202, 254})),
247+
expected: json.NewBlob(string([]byte{202, 254})),
243248
},
244249
{
245-
name: `opaque blob [2 202 254]`,
250+
name: `opaque blob 0xCAFE`,
246251
data: []byte{15, 252, 2, 202, 254},
247-
expected: json.NewBlob(string([]byte{2, 202, 254})),
252+
expected: json.NewBlob(string([]byte{202, 254})),
248253
},
249254
}
250255
for _, tc := range testcases {
@@ -256,6 +261,36 @@ func TestBinaryJSON(t *testing.T) {
256261
}
257262
}
258263

264+
func TestBinaryJSONOpaqueErrors(t *testing.T) {
265+
testcases := []struct {
266+
name string
267+
data []byte
268+
expectedErr string
269+
}{
270+
{
271+
name: "opaque length exceeds payload",
272+
data: []byte{15, 252, 2, 202},
273+
expectedErr: "opaque JSON field value length 2 exceeds available bytes",
274+
},
275+
{
276+
name: "opaque date too short",
277+
data: []byte{15, 10, 4, 0, 0, 0, 0},
278+
expectedErr: "opaque date length 4 is too short",
279+
},
280+
{
281+
name: "opaque decimal too short",
282+
data: []byte{15, 246, 1, 0x01},
283+
expectedErr: "opaque decimal length 1 is too short",
284+
},
285+
}
286+
for _, tc := range testcases {
287+
t.Run(tc.name, func(t *testing.T) {
288+
_, err := ParseBinaryJSON(tc.data)
289+
require.ErrorContains(t, err, tc.expectedErr)
290+
})
291+
}
292+
}
293+
259294
func TestMarshalJSONToSQL(t *testing.T) {
260295
testcases := []struct {
261296
name string
@@ -443,19 +478,20 @@ func TestMarshalJSONToSQL(t *testing.T) {
443478
expected: `CAST(123456789.1234 as JSON)`,
444479
},
445480
{
446-
name: `bit literal [2 202 254]`,
481+
// 0xCAFE = 51966 = binary 1100101011111110 (16 bits)
482+
name: `bit literal 0xCAFE`,
447483
data: []byte{15, 16, 2, 202, 254},
448-
expected: `CAST(b'101100101011111110' as JSON)`,
484+
expected: `CAST(b'1100101011111110' as JSON)`,
449485
},
450486
{
451-
name: `opaque string [2 202 254]`,
487+
name: `opaque string 0xCAFE`,
452488
data: []byte{15, 15, 2, 202, 254},
453-
expected: `CAST(x'02CAFE' as JSON)`,
489+
expected: `CAST(x'CAFE' as JSON)`,
454490
},
455491
{
456-
name: `opaque blob [2 202 254]`,
492+
name: `opaque blob 0xCAFE`,
457493
data: []byte{15, 252, 2, 202, 254},
458-
expected: `CAST(x'02CAFE' as JSON)`,
494+
expected: `CAST(x'CAFE' as JSON)`,
459495
},
460496
}
461497
for _, tc := range testcases {

0 commit comments

Comments
 (0)