Skip to content

Commit 07f50a7

Browse files
Reject extra frames after FailureResponse to prevent misattribution
dqlite's wire protocol is one-request-one-response. A buggy or hostile server emitting two FailureResponses for one EXEC leaves the second buffered in the decoder; the next user request would consume it as if it were that request's response — cross-request misattribution producing a misleading OperationalError on an unrelated operation. In _read_response, after extracting a message: if it's a FailureResponse AND the decoder still has another frame queued, raise ProtocolError. _run_protocol's ProtocolError arm calls _invalidate, the pool discards the slot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d3f5bd9 commit 07f50a7

2 files changed

Lines changed: 85 additions & 0 deletions

File tree

src/dqliteclient/protocol.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,23 @@ async def _read_response(self, deadline: float | None = None) -> Message:
695695
# by code review, not coverage.
696696
raise ProtocolError(f"Failed to decode message{self._addr_suffix()}")
697697

698+
# Hostile-server hardening: a ``FailureResponse`` is always
699+
# terminal per the dqlite wire spec — one request, one
700+
# response. If the decoder still has another frame buffered
701+
# after extracting a FailureResponse, the server emitted
702+
# extra bytes (or two coalesced replies arrived in one TCP
703+
# segment). Without this check the leftover frame would be
704+
# consumed as the response to the NEXT user request,
705+
# producing a misleading ``OperationalError`` against an
706+
# unrelated operation. Raise ``ProtocolError`` here so
707+
# ``_run_protocol`` invalidates the connection and the pool
708+
# discards the slot.
709+
if isinstance(message, FailureResponse) and self._decoder.has_message():
710+
raise ProtocolError(
711+
f"Server emitted extra response after FailureResponse"
712+
f"{self._addr_suffix()} — protocol violation, invalidating connection"
713+
)
714+
698715
return message
699716

700717
def close(self) -> None:
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
"""Pin: a hostile/buggy server emitting two FailureResponses for one
2+
request triggers ``ProtocolError`` and invalidation rather than
3+
silently misattributing the second failure to the next user request.
4+
5+
dqlite's wire protocol is strictly one-request-one-response. A server
6+
that sends two FailureResponses (or any extra frame) for a single
7+
request leaves the second buffered in the decoder; the next user
8+
request would consume it as if it were that request's response —
9+
cross-request misattribution.
10+
11+
The fix: in ``_read_response``, if the just-decoded message is a
12+
``FailureResponse`` AND the decoder still has another frame
13+
buffered, raise ``ProtocolError``. ``_run_protocol``'s
14+
``ProtocolError`` arm calls ``_invalidate`` and the pool discards
15+
the slot.
16+
"""
17+
18+
from __future__ import annotations
19+
20+
from unittest.mock import AsyncMock, MagicMock
21+
22+
import pytest
23+
24+
from dqliteclient.exceptions import ProtocolError
25+
from dqliteclient.protocol import DqliteProtocol
26+
from dqlitewire.messages import FailureResponse
27+
28+
29+
@pytest.fixture
30+
def protocol() -> DqliteProtocol:
31+
reader = AsyncMock()
32+
writer = MagicMock()
33+
writer.drain = AsyncMock()
34+
writer.close = MagicMock()
35+
writer.wait_closed = AsyncMock()
36+
return DqliteProtocol(reader, writer)
37+
38+
39+
@pytest.mark.asyncio
40+
async def test_two_failures_in_a_row_raises_protocol_error(
41+
protocol: DqliteProtocol,
42+
) -> None:
43+
"""Inject two failures in one TCP read; first request raises
44+
ProtocolError (which invalidates) instead of consuming and leaving
45+
the second for cross-misattribution."""
46+
two_failures = (
47+
FailureResponse(code=19, message="first").encode()
48+
+ FailureResponse(code=20, message="second").encode()
49+
)
50+
protocol._reader.read = AsyncMock(side_effect=[two_failures, b""])
51+
with pytest.raises(ProtocolError, match="extra response"):
52+
await protocol.exec_sql(db_id=1, sql="INSERT INTO t VALUES (1)")
53+
54+
55+
@pytest.mark.asyncio
56+
async def test_single_failure_raises_operational_error_not_protocol_error(
57+
protocol: DqliteProtocol,
58+
) -> None:
59+
"""Negative pin: a single FailureResponse (the conforming case)
60+
still surfaces as the existing OperationalError, NOT as
61+
ProtocolError. The hostile-server check is gated on the buffer
62+
having ANOTHER frame queued."""
63+
from dqliteclient.exceptions import OperationalError
64+
65+
one_failure = FailureResponse(code=19, message="constraint failed").encode()
66+
protocol._reader.read = AsyncMock(side_effect=[one_failure, b""])
67+
with pytest.raises(OperationalError, match="constraint failed"):
68+
await protocol.exec_sql(db_id=1, sql="INSERT INTO t VALUES (1)")

0 commit comments

Comments
 (0)