mirror of
https://github.com/caddyserver/caddy.git
synced 2026-11-11 19:37:55 +00:00
Chris Oliver
b3d67c861d
* reverseproxy: set a write deadline on websocket close messages Cleanup sends a Close control message to every attached websocket while holding the tunnel tracker lock, during a config reload. If the peer has stopped reading, the tunnel's copy goroutine is blocked writing to it and the close message waits behind that write forever. The reload never finishes and holds the config lock, so every later /load and /config request hangs until Caddy is restarted. Setting a write deadline before the close message unblocks both writes. The connection is closed right after, so the deadline is never cleared. * Add bot disclosure * reverseproxy: make close control timeout a const, harden stalled-peer test * Delete BOT_DISCLOSURE.md Signed-off-by: Francis Lavoie <lavofr@gmail.com> * reverseproxy: bound each batch of websocket close messages by one deadline The close loops are serial and hold the tunnel tracker lock, so a per-connection write timeout let N stalled peers hold a reload for N times the timeout. Compute one absolute deadline per batch instead, and skip the best-effort close frame on connections that cannot set a write deadline rather than risk the hang. Extended CONNECT streams gain SetWriteDeadline through http.ResponseController. For HTTP/1 upgrades, net/http returns the upstream socket inside a body that hides its deadline methods, so the upstream close message was never bounded; record the connection from the request's ClientTrace and write the close message to that socket. Restores the TODO about being the exclusive writer, per review. * reverseproxy: make websocket close tests deterministic Per review, the stalled-peer tests use net.Pipe instead of real TCP: it is unbuffered and lets one write through at a time, so a close message waits behind a stuck copy write without large allocations or sleeps. The batch test checks that graceful closes share one deadline instead of timing cleanup. The HTTP/1 upstream close test now runs a real upgrade through ServeHTTP and the transport, so it fails if the GotConn hook is removed. The choice of upstream close writer moves into upstreamCloseWriter, with a table test for the HTTP/2 and HTTP/3 case. * reverseproxy: record the upstream conn with atomic.Pointer Per review, store the connection from the GotConn hook in an atomic.Pointer[net.Conn] instead of a mutex-guarded holder type. Rename upstreamCloseWriter to upstreamWriteCloser. It returns io.ReadWriteCloser, since both the body and the recorded socket are one, so gracefulClose keeps its original io.ReadWriteCloser parameter. * reverseproxy: share one close deadline across detached trackers Per review, an upstream removal notifies every stopped tracker in turn, and each close loop holds its own lock, so a deadline per tracker let N trackers with stalled peers hold a reload for N times the timeout. Compute one absolute deadline in notifyDetachedTunnelTrackersOfUpstreamRemoval and pass it through to closeConnectionsForUpstream. Add a test with two stopped trackers holding stalled net.Pipe peers that asserts both graceful closes receive the same deadline. * reverseproxy: simplify the websocket close deadline change Fold the two tunnel close loops into closeConnectionsLocked, so the shared-deadline rule and its rationale live in one place. Replace the per-tracker closeWriteTimeout field, which production never varied, with a package variable that tests shorten. Record the upstream socket only for websocket requests, since only their graceful close reads it, and store a copy of the connection rather than the address of the hook's parameter. Use the package's caddy.CtxKey convention for the context key. Share the upgrade tunnel fixture between the half-close test and the upstream close message test.