From 58864f5ecc3abd351cf252b07995d605e3f7c498 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 5 Aug 2026 12:16:18 +0100 Subject: [PATCH] Accept any inbound websocket frame as proof of client liveness The read deadline was refreshed only by protocol pong frames, so a client whose ping/pong control frames were delayed more than 6 seconds or eaten by a middlebox (Cloudflare edge, AV proxies) was killed every 60 seconds even while its 30-second JSON heartbeats were arriving. Demo logs show the signature clearly: sessions dying at exactly 60s/114s/168s, i.e. zero, one, or two pongs before execution, while the user watched the badge cycle through Reconnecting on an otherwise healthy connection. Refresh the read deadline on every successful read, widen it to 90s so a background tab throttled to one heartbeat per minute still survives, and ping every 30s so pong-only clients tolerate two lost round trips. Contract-Neutral: websocket keepalive hardening: read-deadline/ping cadence only, no payload or contract delta --- internal/websocket/hub.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/internal/websocket/hub.go b/internal/websocket/hub.go index 7fc7f0804..c500be3f5 100644 --- a/internal/websocket/hub.go +++ b/internal/websocket/hub.go @@ -30,6 +30,18 @@ const ( websocketHubComponent = "websocket_hub" initialWelcomeDelay = 500 * time.Millisecond initialStateMessageDelay = 100 * time.Millisecond + // clientReadDeadline is how long a client may stay silent before the server + // drops it. Liveness is proven by ANY inbound frame (data, JSON heartbeat, + // or protocol pong) — proxies and middleboxes (Cloudflare, AV products) + // sometimes eat ping/pong control frames, so pongs must not be the only + // accepted proof of life. Must be comfortably above clientPingPeriod and + // the frontend's 30s JSON heartbeat, including a background tab throttled + // to one heartbeat per minute. + clientReadDeadline = 90 * time.Second + // clientPingPeriod is how often writePump sends protocol pings. At 30s a + // pong-only client survives two consecutive lost ping/pong round trips + // within clientReadDeadline. + clientPingPeriod = 30 * time.Second ) // extractPeerIP extracts just the IP part from a RemoteAddr (host:port format) @@ -1592,12 +1604,12 @@ func (c *Client) readPump() { } }() - if err := c.conn.SetReadDeadline(time.Now().Add(60 * time.Second)); err != nil { + if err := c.conn.SetReadDeadline(time.Now().Add(clientReadDeadline)); err != nil { log.Warn().Err(err).Str("client", c.id).Msg("failed to set initial read deadline") } c.conn.SetReadLimit(maxWebSocketInboundMessageSize) c.conn.SetPongHandler(func(string) error { - if err := c.conn.SetReadDeadline(time.Now().Add(60 * time.Second)); err != nil { + if err := c.conn.SetReadDeadline(time.Now().Add(clientReadDeadline)); err != nil { log.Warn().Err(err).Str("client", c.id).Msg("failed to refresh read deadline on pong") } c.lastPing = time.Now() @@ -1617,6 +1629,13 @@ func (c *Client) readPump() { break } + // Any successful read proves the peer is alive; without this refresh a + // client whose pong frames are lost in transit dies at the deadline even + // while its 30s JSON heartbeats are arriving. + if err := c.conn.SetReadDeadline(time.Now().Add(clientReadDeadline)); err != nil { + log.Warn().Err(err).Str("client", c.id).Msg("failed to refresh read deadline on message") + } + // Handle incoming messages var msg Message if err := json.Unmarshal(message, &msg); err != nil { @@ -1658,7 +1677,7 @@ func (c *Client) writePump() { // Ping deadline can be shorter since pings are small const pingDeadline = 10 * time.Second - ticker := time.NewTicker(54 * time.Second) + ticker := time.NewTicker(clientPingPeriod) defer func() { log.Info().Str("client", c.id).Msg("writePump exiting") ticker.Stop()