Skip to content

Commit 5b7d014

Browse files
Reject trailing bytes in ServersResponse body decode
ServersResponse decoded a variable-length node list but never verified the buffer was fully consumed, silently accepting trailing bytes. This was the only variable-length cluster-topology decoder still permissive after the strict-decode pass across the other fixed-body and variable-length responses. Mirror the strict-decode pattern already applied to FilesResponse and RowsResponse (zero-column fast path): raise DecodeError naming the owning class when the per-node loop leaves bytes unconsumed. Tests pin the exact-body round-trip at 0, 1, and 3 nodes plus trailing-byte rejection in the empty and populated cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent cd778d2 commit 5b7d014

2 files changed

Lines changed: 82 additions & 0 deletions

File tree

src/dqlitewire/messages/responses.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,13 @@ def decode_body(cls, data: bytes, schema: int = 0) -> "ServersResponse":
736736
f"Invalid node role {raw_role} at offset {offset - 8}; expected one of {valid}"
737737
) from exc
738738
nodes.append(NodeInfo(node_id, address, role))
739+
if offset != len(view):
740+
# Strict-decode parity with sibling variable-length
741+
# decoders: conforming Go/C servers never emit trailing
742+
# padding on this response.
743+
raise DecodeError(
744+
f"ServersResponse has {len(view) - offset} trailing bytes after {count} nodes"
745+
)
739746
return cls(nodes)
740747

741748

tests/test_responses_strict_length.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,81 @@ def test_trailing_bytes_rejected(self) -> None:
160160
WelcomeResponse.decode_body(b"\x00" * 9)
161161

162162

163+
class TestServersResponseStrictLength:
164+
"""Variable-length body: ``uint64 count`` then ``count`` × (id, text
165+
address, role). Trailing bytes after the last node had been silently
166+
dropped.
167+
"""
168+
169+
@staticmethod
170+
def _single_node_body(addr: str = "1.2.3.4:9001") -> bytes:
171+
from dqlitewire.types import encode_text, encode_uint64
172+
173+
return (
174+
encode_uint64(1) # count
175+
+ encode_uint64(1) # node_id
176+
+ encode_text(addr) # address (padded)
177+
+ encode_uint64(0) # role = voter
178+
)
179+
180+
def test_empty_list_exact_round_trip(self) -> None:
181+
from dqlitewire.messages.responses import ServersResponse
182+
from dqlitewire.types import encode_uint64
183+
184+
msg = ServersResponse.decode_body(encode_uint64(0))
185+
assert msg.nodes == []
186+
187+
def test_single_node_exact_round_trip(self) -> None:
188+
from dqlitewire.messages.responses import ServersResponse
189+
190+
msg = ServersResponse.decode_body(self._single_node_body())
191+
assert len(msg.nodes) == 1
192+
assert msg.nodes[0].address == "1.2.3.4:9001"
193+
194+
def test_multi_node_exact_round_trip(self) -> None:
195+
"""Offset accumulation through multiple iterations — exercises
196+
the per-node loop boundary condition that the single-node test
197+
cannot reach."""
198+
from dqlitewire.messages.responses import ServersResponse
199+
from dqlitewire.types import encode_text, encode_uint64
200+
201+
body = encode_uint64(3)
202+
for i in range(3):
203+
body += encode_uint64(i)
204+
body += encode_text(f"node{i}.example:900{i}")
205+
body += encode_uint64(0)
206+
msg = ServersResponse.decode_body(body)
207+
assert [n.address for n in msg.nodes] == [
208+
"node0.example:9000",
209+
"node1.example:9001",
210+
"node2.example:9002",
211+
]
212+
213+
def test_trailing_bytes_rejected(self) -> None:
214+
from dqlitewire.messages.responses import ServersResponse
215+
216+
body = self._single_node_body() + b"\x01"
217+
with pytest.raises(DecodeError, match=r"ServersResponse has 1 trailing byte"):
218+
ServersResponse.decode_body(body)
219+
220+
def test_trailing_word_rejected(self) -> None:
221+
from dqlitewire.messages.responses import ServersResponse
222+
223+
body = self._single_node_body() + b"\x00" * 8
224+
with pytest.raises(DecodeError, match=r"ServersResponse has 8 trailing byte"):
225+
ServersResponse.decode_body(body)
226+
227+
def test_empty_list_trailing_bytes_rejected(self) -> None:
228+
"""Count=0 with trailing bytes must still raise — otherwise a
229+
server could hide bytes behind an empty list."""
230+
from dqlitewire.messages.responses import ServersResponse
231+
from dqlitewire.types import encode_uint64
232+
233+
body = encode_uint64(0) + b"\x00" * 8
234+
with pytest.raises(DecodeError, match=r"ServersResponse has 8 trailing byte"):
235+
ServersResponse.decode_body(body)
236+
237+
163238
class TestMetadataResponseStrictLength:
164239
"""Body is two uint64s (failure_domain + weight) — exactly 16 bytes."""
165240

0 commit comments

Comments
 (0)