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 {