* reverseproxy: propagate TCP half-close on upgraded streams
handleUpgradeResponse starts one copy goroutine per direction but returns as
soon as the first one reports a result, and returning runs two unconditional
deferred closes (the client connection and the backend connection). A client
that ends its send direction with a TCP half-close therefore causes the whole
tunnel to be torn down: the backend connection is reset mid-conversation and
whatever it was about to send is never delivered.
Treat a clean EOF as the end of one direction rather than the end of the
tunnel. When a copy finishes without error, propagate the half-close to the
destination with CloseWrite where the connection supports it, and keep waiting
for the other direction so pending bytes can still drain. Real copy errors, the
stream timeout and request cancellation/shutdown still tear down immediately.
If the destination does not support CloseWrite, the peer cannot be told that
the direction ended, so the previous behavior is kept and the tunnel is torn
down rather than held open until the stream timeout.
This is the same class that was fixed upstream in net/http/httputil
(https://go.dev/issue/35892); Caddy has its own upgraded-stream copier, so the
standard library fix does not apply here.
Fixes#8026
* reverseproxy: follow net/http/httputil shape for the half-close
Replace the copyResult struct and the closeWriter/closeWrite helpers with
upstream's plain error channel and errCopyDone sentinel, propagating
CloseWrite inside the copier, as requested in review.
Behaviour is unchanged. The wait is repeated twice inside the existing
select because handleUpgradeResponse also selects on the stream timeout
and sizes the channel at 2 so both goroutines can exit when it fires
(#7418), which net/http/httputil has no equivalent of.
* reverseproxy: explain the repeated wait next to it
Move the reasoning for why the wait is repeated inside the select, rather
than written as upstream's two-line form, into a comment beside the loop
so a future reader does not have to find the review discussion.
* reverseproxy: add an upgraded-stream half-close integration test
Drive a real upgraded stream through the handler and check that closing one
direction is propagated to the other side instead of tearing the tunnel down.
Mirrors net/http/httputil's TestReverseProxyWebSocketHalfTCP, which covers the
same class upstream (https://go.dev/issue/35892): a backend that hijacks and
hands back a *net.TCPConn, the handler in front of it, and a client that dials
it directly, then the four close-read / close-write combinations.
Against the unfixed handler the two half-close cases fail - the surviving
direction never delivers its pending bytes - while the two close-read cases
pass, so the test isolates exactly the reported behaviour.
* reverseproxy: give the half-close test reads a deadline
Without one, a regression blocks in ReadFull until the package timeout
instead of failing the test, which is the flakiness an end-to-end test of
this kind is usually accused of. Ten seconds is far above any real latency
here and turns a hang into a clear failure.
* reverseproxy: hijack the test backend via http.NewResponseController
Matches how the rest of the tree reaches Hijack (responsewriter.go,
caddyauth.go, streaming.go itself) and keeps working if the writer is
wrapped, where a direct http.Hijacker assertion would not.
9be8fb2794