From 1435a77dccad14bc64260fdd605acfe29d13ad1d Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 18 Feb 2026 02:14:29 +0100 Subject: [PATCH] fix(client): fix MessagePack int_64 encoder dead code branch (#516) The int_64 branch condition `-(1 << 63) <= value` overflowed in GDScript's signed 64-bit arithmetic, making the branch unreachable. Negative values beyond int_32 range were incorrectly encoded as uint_64 (0xcf) instead of int_64 (0xd3). Replaced with `value < 0`. Updated boundary tests BV-N15 and BV-N16 to expect correct int_64 header byte. Co-Authored-By: Claude Opus 4.6 --- client/addons/messagepack/messagepack.gd | 2 +- client/tests/test_msgpack_boundaries.gd | 18 ++++++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/client/addons/messagepack/messagepack.gd b/client/addons/messagepack/messagepack.gd index 171c8a8f9..5ada09950 100755 --- a/client/addons/messagepack/messagepack.gd +++ b/client/addons/messagepack/messagepack.gd @@ -87,7 +87,7 @@ static func _encode_message(buffer: StreamPeerBuffer, value): elif 0 <= value and value <= (1 << 32) - 1: buffer.put_u8(types["uint_32"]) buffer.put_u32(value) - elif - (1 << 63) <= value and value < (1 << 63): + elif value < 0: buffer.put_u8(types["int_64"]) buffer.put_64(value) else: diff --git a/client/tests/test_msgpack_boundaries.gd b/client/tests/test_msgpack_boundaries.gd index a2b9822d1..eb1501fd0 100644 --- a/client/tests/test_msgpack_boundaries.gd +++ b/client/tests/test_msgpack_boundaries.gd @@ -85,13 +85,8 @@ func test_encode_uint32() -> void: func test_encode_int64_positive() -> void: - # BV-P24 to BV-P25: int 64 range (2^32 to 2^63-1) - # KNOWN-DEFECT: The encoder's int_64 branch condition `-(1 << 63) <= v < (1 << 63)` - # evaluates to `MIN_INT64 <= v < MIN_INT64` due to overflow, making it dead code. - # Values that should be int_64 (0xd3) are instead encoded as uint_64 (0xcf). - # Roundtrip still works because put_u64/get_u64 preserve the bit pattern. - # This test documents ACTUAL behavior. Fix tracked in backlog. - # See: messagepack.gd int_64 branch — use explicit constant instead of `1 << 63`. + # BV-P24 to BV-P25: positive values > uint_32 max → uint_64 (0xcf) + # Positive values correctly use uint_64 encoding for Rust interop. _assert_encodes_to(4294967296, PackedByteArray([ 0xcf, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00 ]), "BV-P24") @@ -138,16 +133,15 @@ func test_encode_int32_negative() -> void: func test_encode_int64_negative() -> void: # BV-N15 to BV-N16: int 64 range (< -2147483648) - # KNOWN-DEFECT: Same int_64 branch issue as positive int_64 — encoded as uint_64 (0xcf). - # Bit pattern is preserved: put_u64(negative) writes two's complement, - # get_u64() reads it back and Variant stores as int64 with same bit pattern. + # Negative values beyond int_32 now correctly encode as int_64 (0xd3). + # Fixed: encoder's int_64 branch used `-(1 << 63)` which overflowed to dead code. _assert_encodes_to(-2147483649, PackedByteArray([ - 0xcf, 0xff, 0xff, 0xff, 0xff, 0x7f, 0xff, 0xff, 0xff + 0xd3, 0xff, 0xff, 0xff, 0xff, 0x7f, 0xff, 0xff, 0xff ]), "BV-N15") # MIN_INT64: -9223372036854775808 var min_int64: int = -9223372036854775807 - 1 _assert_encodes_to(min_int64, PackedByteArray([ - 0xcf, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + 0xd3, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 ]), "BV-N16")