Skip to content

Commit f4550cd

Browse files
Apply max_continuation_frames cap to the interrupt() drain loop
interrupt() drained in-flight continuation frames after sending INTERRUPT but only enforced the operation deadline — not the max_continuation_frames cap that _drain_continuations uses on the query path. A slow-dripping server answering an INTERRUPT with a steady stream of small RowsResponse frames could pin the client on per-frame decode work within a single deadline, reaching the DoS scenario the cap was introduced to bound. Count every RowsResponse (including has_more=False "done marker" frames) and raise ProtocolError when the cap trips, matching the shape of the sibling check in _drain_continuations. The operation-deadline guard is preserved; callers who pass max_continuation_frames=None keep the existing deadline-only semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 81227c8 commit f4550cd

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

src/dqliteclient/protocol.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,12 @@ async def interrupt(self, db_id: int) -> None:
263263

264264
# Drain: swallow any trailing continuation frames, break when
265265
# EmptyResponse arrives. Bound by the single operation deadline
266-
# so a non-responsive server cannot stall this forever.
266+
# so a non-responsive server cannot stall this forever, and by
267+
# the max_continuation_frames cap so a slow-dripping server
268+
# cannot pin the client on per-frame decode work inside that
269+
# deadline window (same rationale as _drain_continuations).
267270
deadline = self._operation_deadline()
271+
frames = 0
268272
while True:
269273
response = await self._read_response(deadline=deadline)
270274
if isinstance(response, EmptyResponse):
@@ -279,6 +283,13 @@ async def interrupt(self, db_id: int) -> None:
279283
f"Expected EmptyResponse after Interrupt, got "
280284
f"{type(response).__name__}{self._addr_suffix()}"
281285
)
286+
frames += 1
287+
if self._max_continuation_frames is not None and frames > self._max_continuation_frames:
288+
raise ProtocolError(
289+
f"Interrupt drain exceeded max_continuation_frames cap "
290+
f"({self._max_continuation_frames}); server may be "
291+
f"slow-dripping rows{self._addr_suffix()}."
292+
)
282293
# If the RowsResponse signals more frames, keep draining.
283294
if not response.has_more:
284295
# DONE marker arrived: some servers emit the final

tests/test_interrupt.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,76 @@ async def test_interrupt_raises_operational_error_on_failure(
7575
await protocol.interrupt(db_id=1)
7676
assert exc_info.value.code == 5
7777

78+
async def test_interrupt_drain_respects_max_continuation_frames(self) -> None:
79+
"""The drain loop must honour the same max_continuation_frames
80+
cap that _drain_continuations uses on the query path.
81+
Otherwise a slow-dripping server answering an INTERRUPT with
82+
many small RowsResponse frames can pin a client within a
83+
single operation deadline.
84+
"""
85+
reader = AsyncMock()
86+
writer = MagicMock()
87+
writer.drain = AsyncMock()
88+
writer.close = MagicMock()
89+
writer.wait_closed = AsyncMock()
90+
proto = DqliteProtocol(
91+
reader,
92+
writer,
93+
timeout=10.0,
94+
address="test:9001",
95+
max_continuation_frames=3,
96+
)
97+
proto._handshake_done = True
98+
99+
# Four in-flight frames arrive before EmptyResponse; cap = 3
100+
# trips on frame 4. RowsResponse with has_more=False is the
101+
# canonical "done marker" shape the drain loop already swallows.
102+
rows_frame = RowsResponse(
103+
column_names=["x"],
104+
column_types=[ValueType.INTEGER],
105+
rows=[(1,)],
106+
row_types=[(ValueType.INTEGER,)],
107+
has_more=False,
108+
).encode()
109+
empty = EmptyResponse().encode()
110+
proto._reader.read = AsyncMock( # type: ignore[attr-defined]
111+
side_effect=[rows_frame * 4 + empty, b""]
112+
)
113+
with pytest.raises(ProtocolError, match="max_continuation_frames"):
114+
await proto.interrupt(db_id=1)
115+
116+
async def test_interrupt_drain_no_cap_when_governor_unset(self) -> None:
117+
"""max_continuation_frames=None restores the existing behaviour
118+
(bound only by the operation deadline). Regression guard for
119+
callers that opt out of the cap."""
120+
reader = AsyncMock()
121+
writer = MagicMock()
122+
writer.drain = AsyncMock()
123+
writer.close = MagicMock()
124+
writer.wait_closed = AsyncMock()
125+
proto = DqliteProtocol(
126+
reader,
127+
writer,
128+
timeout=10.0,
129+
address="test:9001",
130+
max_continuation_frames=None,
131+
)
132+
proto._handshake_done = True
133+
134+
rows_frame = RowsResponse(
135+
column_names=["x"],
136+
column_types=[ValueType.INTEGER],
137+
rows=[(1,)],
138+
row_types=[(ValueType.INTEGER,)],
139+
has_more=False,
140+
).encode()
141+
empty = EmptyResponse().encode()
142+
proto._reader.read = AsyncMock( # type: ignore[attr-defined]
143+
side_effect=[rows_frame * 10 + empty, b""]
144+
)
145+
# No raise: deadline-only behaviour preserved.
146+
await proto.interrupt(db_id=1)
147+
78148
async def test_interrupt_raises_protocol_error_on_unexpected_message(
79149
self, protocol: DqliteProtocol
80150
) -> None:

0 commit comments

Comments
 (0)