Skip to content

Commit 210c263

Browse files
Pin handshake _read_timeout opt-in/opt-out and cap behaviour
The widen path runs as the very last step of handshake, so torn state is impossible — but the contract still has to hold under extreme advertised values: * trust_server_heartbeat=False (default) keeps the operator's configured timeout regardless of advertisement. * trust=True widens up to _HEARTBEAT_READ_TIMEOUT_CAP_SECONDS but never narrows. * heartbeat <= 0 disables widening (the > 0 guard) so a malformed advertisement is a no-op, not a quietly logged widen. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 90947f2 commit 210c263

1 file changed

Lines changed: 92 additions & 0 deletions

File tree

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
"""Pin: ``handshake`` either widens ``_read_timeout`` atomically (when
2+
opted in) or leaves the operator-configured value unchanged.
3+
4+
Two contracts:
5+
6+
1. ``trust_server_heartbeat=False`` (default): the server-advertised
7+
heartbeat is recorded for diagnostics but MUST NOT widen
8+
``_read_timeout``. A regression that always widens would let a
9+
hostile server stretch the operator's read SLO up to 300 s.
10+
2. ``trust_server_heartbeat=True``: widens up to the
11+
``_HEARTBEAT_READ_TIMEOUT_CAP_SECONDS`` cap and never narrows.
12+
13+
The widen is the very last write before ``handshake`` returns, so
14+
torn-state is impossible — but the contract still has to hold under
15+
extreme advertised values (negative, zero, huge).
16+
"""
17+
18+
from __future__ import annotations
19+
20+
from unittest.mock import AsyncMock, MagicMock
21+
22+
import pytest
23+
24+
from dqliteclient.protocol import _HEARTBEAT_READ_TIMEOUT_CAP_SECONDS, DqliteProtocol
25+
from dqlitewire.messages import WelcomeResponse
26+
27+
28+
def _make_protocol(timeout: float, *, trust: bool) -> DqliteProtocol:
29+
reader = MagicMock()
30+
writer = MagicMock()
31+
writer.write = MagicMock()
32+
writer.drain = AsyncMock()
33+
return DqliteProtocol(reader, writer, timeout=timeout, trust_server_heartbeat=trust)
34+
35+
36+
@pytest.mark.asyncio
37+
async def test_default_handshake_does_not_widen_read_timeout() -> None:
38+
"""Pin: with ``trust_server_heartbeat=False`` (default), a server
39+
advertising a 300 s heartbeat does NOT widen the operator's
40+
configured per-read deadline."""
41+
proto = _make_protocol(timeout=5.0, trust=False)
42+
proto._send = AsyncMock()
43+
proto._read_response = AsyncMock(return_value=WelcomeResponse(heartbeat_timeout=300_000))
44+
45+
await proto.handshake()
46+
47+
assert proto._read_timeout == 5.0
48+
# Heartbeat is recorded for diagnostics either way.
49+
assert proto._heartbeat_timeout == 300_000
50+
51+
52+
@pytest.mark.asyncio
53+
async def test_trust_handshake_widens_up_to_cap() -> None:
54+
"""Pin: with ``trust=True`` and a server heartbeat above the cap,
55+
``_read_timeout`` widens to the cap and not beyond."""
56+
proto = _make_protocol(timeout=5.0, trust=True)
57+
proto._send = AsyncMock()
58+
# Advertise 600 s; cap is 300.
59+
proto._read_response = AsyncMock(return_value=WelcomeResponse(heartbeat_timeout=600_000))
60+
61+
await proto.handshake()
62+
63+
assert proto._read_timeout == _HEARTBEAT_READ_TIMEOUT_CAP_SECONDS
64+
65+
66+
@pytest.mark.asyncio
67+
async def test_trust_handshake_does_not_narrow_read_timeout() -> None:
68+
"""Pin: a small server heartbeat (e.g., 1 s) does not narrow the
69+
operator's larger configured timeout."""
70+
proto = _make_protocol(timeout=30.0, trust=True)
71+
proto._send = AsyncMock()
72+
proto._read_response = AsyncMock(return_value=WelcomeResponse(heartbeat_timeout=1_000))
73+
74+
await proto.handshake()
75+
76+
assert proto._read_timeout == 30.0
77+
78+
79+
@pytest.mark.asyncio
80+
async def test_trust_handshake_zero_or_negative_heartbeat_no_widen() -> None:
81+
"""Pin: heartbeat <= 0 disables widening (the ``> 0`` guard).
82+
Without it a malformed/zero advertisement would route through the
83+
widen math and produce ``_read_timeout = max(t, 0) = t`` —
84+
behaviourally a no-op today, but the explicit guard keeps the
85+
diagnostic logging accurate."""
86+
proto = _make_protocol(timeout=5.0, trust=True)
87+
proto._send = AsyncMock()
88+
proto._read_response = AsyncMock(return_value=WelcomeResponse(heartbeat_timeout=0))
89+
90+
await proto.handshake()
91+
92+
assert proto._read_timeout == 5.0

0 commit comments

Comments
 (0)