Validate reserved fields on Header, DbResponse, EmptyResponse decode

LeaderRequest.decode_body validates its body-level reserved uint64 is
zero; the corresponding Header, DbResponse, and EmptyResponse
decoders did not — three reserved words the upstream C protocol pins
to zero were silently accepted or not read at all. Asymmetry blocks
using those bits as a future schema-version signal and hides peer
corruption that should surface as a clean DecodeError.

Apply symmetric strict validation:

- Header.decode: reject non-zero `reserved` uint16 (message.h's `extra`
  field).
- DbResponse.decode_body: require the full 8-byte body, read the
  trailing uint32 reserved field, reject non-zero.
- EmptyResponse.decode_body: require the full 8-byte body, read the
  uint64 reserved, reject non-zero.

Error wording mirrors LeaderRequest's phrasing
("<MessageName> reserved field must be 0, got {reserved}") for
consistency.

Updated tests:

- TestHeaderReservedField: add test_decode_rejects_nonzero_reserved.
- TestReservedFieldValidation (new): positive and negative cases for
  DbResponse and EmptyResponse.
- test_decode_body_raises_on_short: DbResponse min body bumped from
  4 to 8, EmptyResponse added (8).
- The prior test_empty_response_accepts_any_body pinned the lax
  behaviour and is replaced by the new validation tests.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: antoineleclair

src/dqlitewire/messages/base.py

@@ -47,6 +47,13 @@ def decode(cls, data: bytes) -> "Header":
             size_words, msg_type, schema, reserved = struct.unpack("<IBBH", data[:HEADER_SIZE])
         except struct.error as e:
             raise DecodeError(f"Failed to decode header: {e}") from e
+        # Upstream C (message.h) reserves the trailing uint16 and every
+        # current server writes 0. Reject non-zero values so peer
+        # corruption or a future schema extension surfaces as a clean
+        # DecodeError instead of silently carrying bits we cannot
+        # re-emit. Matches LeaderRequest.decode_body's strict check.
+        if reserved != 0:
+            raise DecodeError(f"Header reserved field must be 0, got {reserved}")
         return cls(size_words, msg_type, schema, reserved)
 
     @property

src/dqlitewire/messages/responses.py

@@ -143,7 +143,12 @@ def encode_body(self) -> bytes:
 
     @classmethod
     def decode_body(cls, data: bytes, schema: int = 0) -> "DbResponse":
+        if len(data) < 8:
+            raise DecodeError(f"DbResponse body must be 8 bytes, got {len(data)}")
         db_id = decode_uint32(data)
+        reserved = decode_uint32(data[4:])
+        if reserved != 0:
+            raise DecodeError(f"DbResponse reserved field must be 0, got {reserved}")
         return cls(db_id)
 
 
@@ -472,6 +477,11 @@ def encode_body(self) -> bytes:
 
     @classmethod
     def decode_body(cls, data: bytes, schema: int = 0) -> "EmptyResponse":
+        if len(data) < 8:
+            raise DecodeError(f"EmptyResponse body must be 8 bytes, got {len(data)}")
+        reserved = decode_uint64(data)
+        if reserved != 0:
+            raise DecodeError(f"EmptyResponse reserved field must be 0, got {reserved}")
         return cls()
 
 

tests/test_messages_requests.py

@@ -50,6 +50,25 @@ def test_decoded_reserved_is_zero_after_encode_decode(self) -> None:
         header = Header.decode(msg.encode()[:HEADER_SIZE])
         assert header.reserved == 0
 
+    def test_decode_rejects_nonzero_reserved(self) -> None:
+        """Header.decode must reject non-zero reserved bytes.
+
+        The protocol spec pins the trailing uint16 at 0; a non-zero value
+        signals peer corruption, a buggy encoder, or a future schema
+        extension we do not understand. Reject it cleanly rather than
+        silently carrying it past a boundary that can never re-emit it.
+        """
+        import struct
+
+        import pytest
+
+        from dqlitewire.exceptions import DecodeError
+
+        # size_words=1, msg_type=0x42 (arbitrary), schema=0, reserved=0xBEEF
+        encoded = struct.pack("<IBBH", 1, 0x42, 0, 0xBEEF)
+        with pytest.raises(DecodeError, match="reserved field must be 0"):
+            Header.decode(encoded)
+
 
 class TestLeaderRequest:
     def test_encode_has_body(self) -> None:

tests/test_messages_responses.py

@@ -1294,7 +1294,8 @@ class TestShortBodyDecoding:
             (WelcomeResponse, 8),  # uint64 heartbeat_timeout
             (MetadataResponse, 16),  # 2 x uint64
             (ResultResponse, 16),  # 2 x uint64
-            (DbResponse, 4),  # uint32 db_id
+            (DbResponse, 8),  # uint32 db_id + uint32 reserved
+            (EmptyResponse, 8),  # uint64 reserved
         ],
     )
     def test_decode_body_raises_on_short(self, cls: type, min_body: int) -> None:
@@ -1309,12 +1310,30 @@ def test_leader_response_legacy_missing_null_terminator_raises(self) -> None:
         with pytest.raises(DecodeError):
             LeaderResponse.decode_body_legacy(b"abc")
 
-    def test_empty_response_accepts_any_body(self) -> None:
-        """``EmptyResponse`` is documented as having an 8-byte reserved
-        field but its decoder accepts any body length today. Pin the
-        current (lax) behaviour so any future tightening is a deliberate
-        decision — not a silent refactor side-effect.
-        """
-        assert isinstance(EmptyResponse.decode_body(b""), EmptyResponse)
+
+class TestReservedFieldValidation:
+    """Non-zero reserved bytes must fail to decode across all message
+    types that declare a reserved field, matching LeaderRequest's
+    strict posture. Silently accepting non-zero reserved would hide
+    peer corruption and block reusing those bits for any future schema
+    signal.
+    """
+
+    def test_db_response_rejects_nonzero_reserved(self) -> None:
+        # uint32 db_id + uint32 reserved (0xFFFFFFFF)
+        body = b"\x01\x00\x00\x00" + b"\xff\xff\xff\xff"
+        with pytest.raises(DecodeError, match="DbResponse reserved field must be 0"):
+            DbResponse.decode_body(body)
+
+    def test_db_response_accepts_zero_reserved(self) -> None:
+        # Round-trip form stays accepted.
+        body = b"\x01\x00\x00\x00" + b"\x00\x00\x00\x00"
+        assert DbResponse.decode_body(body).db_id == 1
+
+    def test_empty_response_rejects_nonzero_reserved(self) -> None:
+        body = b"\xff" * 8
+        with pytest.raises(DecodeError, match="EmptyResponse reserved field must be 0"):
+            EmptyResponse.decode_body(body)
+
+    def test_empty_response_accepts_zero_reserved(self) -> None:
         assert isinstance(EmptyResponse.decode_body(b"\x00" * 8), EmptyResponse)
-        assert isinstance(EmptyResponse.decode_body(b"\xff" * 64), EmptyResponse)