Skip to content

Commit 729228d

Browse files
Reject trailing bytes in fixed-length response decoders
EmptyResponse and DbResponse used len(data) < expected guards, which silently discarded any trailing bytes past the first 8 — a permissive parse asymmetric with peers like LeaderRequest.decode_body that already enforce exact-length. ResultResponse had no up-front length check at all: truncated bodies raised a generic "Need 8 bytes for uint64" from the primitive decoder (losing the ResultResponse frame name in the message), and oversized bodies silently succeeded. Tighten all three to len(data) != N with the message naming the framed type, and add a strict-length test suite covering short, exact, trailing-bytes, and trailing-zero inputs so future regressions fail loudly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 248214a commit 729228d

2 files changed

Lines changed: 86 additions & 4 deletions

File tree

src/dqlitewire/messages/responses.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,8 @@ def encode_body(self) -> bytes:
211211

212212
@classmethod
213213
def decode_body(cls, data: bytes, schema: int = 0) -> "DbResponse":
214-
if len(data) < 8:
215-
raise DecodeError(f"DbResponse body must be 8 bytes, got {len(data)}")
214+
if len(data) != 8:
215+
raise DecodeError(f"DbResponse body must be exactly 8 bytes, got {len(data)}")
216216
db_id = decode_uint32(data)
217217
reserved = decode_uint32(data[4:])
218218
if reserved != 0:
@@ -298,6 +298,8 @@ def encode_body(self) -> bytes:
298298

299299
@classmethod
300300
def decode_body(cls, data: bytes, schema: int = 0) -> "ResultResponse":
301+
if len(data) != 16:
302+
raise DecodeError(f"ResultResponse body must be exactly 16 bytes, got {len(data)}")
301303
last_insert_id = decode_uint64(data)
302304
rows_affected = decode_uint64(data[8:])
303305
return cls(last_insert_id, rows_affected)
@@ -558,8 +560,8 @@ def encode_body(self) -> bytes:
558560

559561
@classmethod
560562
def decode_body(cls, data: bytes, schema: int = 0) -> "EmptyResponse":
561-
if len(data) < 8:
562-
raise DecodeError(f"EmptyResponse body must be 8 bytes, got {len(data)}")
563+
if len(data) != 8:
564+
raise DecodeError(f"EmptyResponse body must be exactly 8 bytes, got {len(data)}")
563565
reserved = decode_uint64(data)
564566
if reserved != 0:
565567
raise DecodeError(f"EmptyResponse reserved field must be 0, got {reserved}")
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
"""Fixed-length response decoders must reject trailing bytes.
2+
3+
Strict-parse peers in this module (``LeaderRequest.decode_body``,
4+
``StmtResponse.decode_body``) assert exact body lengths. The three
5+
messages audited here previously accepted ``len >= expected``,
6+
silently ignoring trailing bytes — asymmetric with peers and
7+
permissive in a way that masks frame-corruption.
8+
9+
Peers of ISSUE-298 / ISSUE-299.
10+
"""
11+
12+
from __future__ import annotations
13+
14+
import pytest
15+
16+
from dqlitewire.exceptions import DecodeError
17+
from dqlitewire.messages.responses import DbResponse, EmptyResponse, ResultResponse
18+
19+
20+
class TestEmptyResponseStrictLength:
21+
def test_short_body_rejected(self) -> None:
22+
with pytest.raises(DecodeError, match="EmptyResponse body must be exactly 8 bytes"):
23+
EmptyResponse.decode_body(b"\x00" * 7)
24+
25+
def test_exact_length_accepted(self) -> None:
26+
msg = EmptyResponse.decode_body(b"\x00" * 8)
27+
assert isinstance(msg, EmptyResponse)
28+
29+
def test_trailing_bytes_rejected(self) -> None:
30+
"""16-byte body: decoders used to silently discard the
31+
trailing 8 bytes. Must now raise.
32+
"""
33+
with pytest.raises(DecodeError, match="EmptyResponse body must be exactly 8 bytes"):
34+
EmptyResponse.decode_body(b"\x00" * 16)
35+
36+
def test_trailing_zero_bytes_still_rejected(self) -> None:
37+
"""Trailing zeros look innocuous but must still fail — the
38+
decoder should not need to introspect the padding."""
39+
with pytest.raises(DecodeError, match="EmptyResponse body must be exactly 8 bytes"):
40+
EmptyResponse.decode_body(b"\x00" * 9)
41+
42+
def test_reserved_nonzero_still_rejected(self) -> None:
43+
"""Length check comes first; reserved-field check must still
44+
apply on exactly-8-byte bodies."""
45+
with pytest.raises(DecodeError, match="reserved field must be 0"):
46+
EmptyResponse.decode_body(b"\x01" + b"\x00" * 7)
47+
48+
49+
class TestDbResponseStrictLength:
50+
def test_short_body_rejected(self) -> None:
51+
with pytest.raises(DecodeError, match="DbResponse body must be exactly 8 bytes"):
52+
DbResponse.decode_body(b"\x00" * 7)
53+
54+
def test_exact_length_accepted(self) -> None:
55+
msg = DbResponse.decode_body(b"\x05\x00\x00\x00" + b"\x00" * 4)
56+
assert isinstance(msg, DbResponse)
57+
assert msg.db_id == 5
58+
59+
def test_trailing_bytes_rejected(self) -> None:
60+
with pytest.raises(DecodeError, match="DbResponse body must be exactly 8 bytes"):
61+
DbResponse.decode_body(b"\x00" * 16)
62+
63+
64+
class TestResultResponseStrictLength:
65+
def test_short_body_rejected_with_type_name(self) -> None:
66+
"""Error message names ``ResultResponse``, not just
67+
``uint64`` — so operators reading logs can trace the
68+
framed message."""
69+
with pytest.raises(DecodeError, match="ResultResponse body must be exactly 16 bytes"):
70+
ResultResponse.decode_body(b"\x00" * 15)
71+
72+
def test_exact_length_accepted(self) -> None:
73+
body = (42).to_bytes(8, "little") + (7).to_bytes(8, "little")
74+
msg = ResultResponse.decode_body(body)
75+
assert msg.last_insert_id == 42
76+
assert msg.rows_affected == 7
77+
78+
def test_trailing_bytes_rejected(self) -> None:
79+
with pytest.raises(DecodeError, match="ResultResponse body must be exactly 16 bytes"):
80+
ResultResponse.decode_body(b"\x00" * 17)

0 commit comments

Comments
 (0)