From 68244d806cb8b7ffe59d91028c4ac8e166bf96e8 Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 20 Oct 2026 13:18:48 +0000 Subject: [PATCH] An upgrade the app accepts under another protocol name is answered 502 (closes #131) ReverseProxy refuses a 101 whose Upgrade is not the one the client asked for, but modifyResponse had already counted the 101 as sent, so answerError sent nothing and the client got an empty 200. The 101 now counts as sent only once ReverseProxy has taken over the client's connection, and answerError closes the app's switched connection, which ReverseProxy leaves open when it refuses the switch. Model: opus-5-5 --- internal/proxy/bodies.go | 17 +++++++++++++++-- internal/proxy/passthrough_test.go | 17 +++++++++++++++++ internal/proxy/request.go | 11 ++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/internal/proxy/bodies.go b/internal/proxy/bodies.go index ef7110a..3180b6b 100644 --- a/internal/proxy/bodies.go +++ b/internal/proxy/bodies.go @@ -1,8 +1,10 @@ package proxy import ( + "bufio" "errors" "io" + "net" "net/http" "sync/atomic" @@ -211,9 +213,20 @@ func (w *responseWriter) FlushError() error { return err } +// Hijack takes over the connection to the client, which ReverseProxy does +// only to pass on the app's 101 Switching Protocols: that is then the +// status sent. +func (w *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { + conn, buffered, err := http.NewResponseController(w.ResponseWriter).Hijack() + if err == nil { + w.status = http.StatusSwitchingProtocols + } + + return conn, buffered, err +} + // Unwrap lets http.ResponseController reach net/http's own -// ResponseWriter, which is how ReverseProxy takes over the connection of -// an upgraded request. +// ResponseWriter for what responseWriter does not do itself. func (w *responseWriter) Unwrap() http.ResponseWriter { return w.ResponseWriter } diff --git a/internal/proxy/passthrough_test.go b/internal/proxy/passthrough_test.go index aade4f6..46cb72d 100644 --- a/internal/proxy/passthrough_test.go +++ b/internal/proxy/passthrough_test.go @@ -309,6 +309,23 @@ func echoAfterUpgrade(w http.ResponseWriter, r *http.Request) { } } +func TestAnswers502WhenTheAppSwitchesToAnotherProtocol(t *testing.T) { + t.Parallel() + + // answerAfterUpgrade switches to websocket whatever the client asked + // for, and ReverseProxy passes on a switch only to the protocol the + // client asked for, here "websocket, foo". + app := startApp(t, answerAfterUpgrade) + addr, out := startProxy(t, app.URL, nil) + + conn := dial(t, addr) + send(t, conn, "GET /ws HTTP/1.1\r\nHost: app\r\n"+ + "Connection: Upgrade\r\nUpgrade: websocket, foo\r\n\r\n") + + wantStatus(t, readResponse(t, conn), http.StatusBadGateway) + wantLine(t, out.requestLine(t), http.StatusBadGateway, requestlog.ActionUpstreamError) +} + func TestServerHasTheDefaultLimits(t *testing.T) { t.Parallel() diff --git a/internal/proxy/request.go b/internal/proxy/request.go index 8ddd13a..e67599c 100644 --- a/internal/proxy/request.go +++ b/internal/proxy/request.go @@ -395,8 +395,10 @@ func (rq *request) modifyResponse(res *http.Response) error { // timeouts. ReverseProxy writes this answer straight to the // connection it takes over, not through rq.out, and then copies // what passes each way through res.Body, the connection to the app. + // It may still refuse the switch, as when the app's Upgrade is not + // the client's, so the status is noted only once it has taken over + // the connection, by rq.out's Hijack. rq.stopTimers() - rq.out.status = res.StatusCode rq.line.Websocket = true conn, ok := res.Body.(io.ReadWriteCloser) @@ -428,6 +430,13 @@ func (rq *request) modifyResponse(res *http.Response) error { // answerError is ReverseProxy's ErrorHandler: the request could not be // passed to the app, or the app's answer cannot be passed on. func (rq *request) answerError(_ http.ResponseWriter, _ *http.Request, err error) { + if rq.upgraded != nil { + // The app switched protocols, but the switch was not passed on, + // and ReverseProxy leaves the app's connection open when it + // refuses it. + _ = rq.upgraded.Close() + } + refused := rq.refused.Load() if refused == nil { if rq.in.Context().Err() != nil { -- 2.54.0