Skip to content

Commit d241d87

Browse files
Document deliberate opt-out from wire skip_message recovery affordance
The wire layer's MessageDecoder.skip_message + ReadBuffer's deferred-discard machinery is documented as a recovery affordance for oversize-message rejections (read_message raises DecodeError without poisoning so a caller can drain the over-large frame and resume on the same connection). DqliteProtocol never invokes it — every wire-level ProtocolError is wrapped to a client-level ProtocolError and routed through _invalidate to drop the connection. The opt-out is deliberate (a 64 MiB cap is high enough that hitting it almost always indicates an attacker, a misbehaving server, or a deeply-nested wire bug — none safe to resume from on the same socket; the pool's re-acquire path is the right recovery), but it was undocumented. A reader of the wire docstring would expect skip_message to be invoked somewhere, and a future contributor might quietly add it without re-evaluating the security posture. Document the rationale at the wire-DecodeError wrap site in _read_response, name skip_message explicitly so grep finds it, and pin the opt-out (no skip_message / is_skipping calls anywhere in the client module) against accidental re-introduction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4c31cf0 commit d241d87

2 files changed

Lines changed: 71 additions & 0 deletions

File tree

src/dqliteclient/protocol.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -861,6 +861,25 @@ async def _read_continuation(self, deadline: float | None = None) -> RowsRespons
861861
e.code, f"{e.message}{self._addr_suffix()}", raw_message=e.message
862862
) from e
863863
except _WireProtocolError as e:
864+
# Wire-decode failures are NOT recovered via
865+
# ``MessageDecoder.skip_message``. The wire layer documents
866+
# oversize-message rejection as recoverable in-place
867+
# (``read_message`` raises ``DecodeError`` without poisoning
868+
# so a caller can drain the over-large frame via
869+
# ``skip_message`` + ``feed`` and resume on the same
870+
# connection). The client layer deliberately opts out: we
871+
# wrap every wire-level ProtocolError into a client-level
872+
# ProtocolError, which ``_run_protocol`` routes through
873+
# ``_invalidate`` to drop the connection. Rationale: a
874+
# 64 MiB cap is high enough that hitting it almost always
875+
# indicates an attacker, a misbehaving server, or a
876+
# deeply-nested wire bug — none of which the client can
877+
# safely resume from on the same socket. The pool's
878+
# re-acquire path (handshake + OPEN) is the right
879+
# recovery, not in-place skip. The ``skip_message`` API
880+
# remains available for third-party harnesses that
881+
# consume ``DqliteProtocol`` directly and want a
882+
# different policy.
864883
raise ProtocolError(f"Wire decode failed{self._addr_suffix()}: {e}") from e
865884

866885
async def _read_response(self, deadline: float | None = None) -> Message:
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
"""Pin: ``DqliteProtocol`` deliberately does NOT invoke
2+
``skip_message`` to recover from oversize-message rejections — every
3+
wire-level ProtocolError (including the recoverable, non-poisoning
4+
oversize DecodeError) is wrapped to a client-level ProtocolError and
5+
routed through ``_invalidate`` to drop the connection.
6+
7+
The wire layer documents oversize as a recoverable-in-place
8+
affordance (``read_message`` raises without poisoning; ``skip_message``
9+
drains). The client layer opts out for security (a 64 MiB cap is
10+
high enough that hitting it almost always indicates an attacker, a
11+
misbehaving server, or a wire bug — none safe to resume from on the
12+
same socket). Pin so a future contributor reading the wire-layer
13+
docstring doesn't quietly add ``skip_message`` calls into the client
14+
without re-evaluating the security posture.
15+
"""
16+
17+
from __future__ import annotations
18+
19+
import inspect
20+
21+
from dqliteclient import protocol as proto_mod
22+
23+
24+
def test_protocol_module_does_not_invoke_skip_message() -> None:
25+
"""Source-level pin: no client-layer code path calls
26+
``skip_message`` or reads ``is_skipping``. A regression that
27+
re-introduces the recovery would show up here."""
28+
src = inspect.getsource(proto_mod)
29+
assert "skip_message" not in src or "skip_message`` API" in src, (
30+
"DqliteProtocol must not invoke skip_message — wire-level "
31+
"DecodeError must drop the connection per the deliberate "
32+
"opt-out documented in the _read_response wrap site."
33+
)
34+
assert "is_skipping" not in src, (
35+
"DqliteProtocol must not read is_skipping — the skip-recovery path is unused by design."
36+
)
37+
38+
39+
def test_protocol_documents_skip_message_opt_out_rationale() -> None:
40+
"""The ``_read_response`` wrap site (or its sibling) must contain
41+
a comment block documenting the deliberate opt-out, so a future
42+
reader of the wire layer's recoverable-oversize docstring is
43+
redirected to the client-layer policy instead of quietly adding
44+
skip_message calls."""
45+
src = inspect.getsource(proto_mod)
46+
assert "deliberately opts out" in src or "deliberate opt-out" in src.lower(), (
47+
"The protocol module should document the skip_message opt-out "
48+
"near the wire-DecodeError wrap site"
49+
)
50+
assert "skip_message" in src, (
51+
"The opt-out comment should name skip_message so a future reader can find it via grep"
52+
)

0 commit comments

Comments
 (0)