net/http/httputil: ReverseProxy fails to propagate close on hijacked connection with replaced response body

[neild]

This is a regression in 1.24 caused by CL 637939 for #35892.

That CL changes httputil.ReverseProxy to propagate the half-closed state of hijacked connections. For example, if a client creates a WebSocket connection through a proxy and write-closes its end of the connection, ReverseProxy will write-close its connection to the backend.

The problem occurs when the backend connection (as represented by a Response.Body) does not have a WriteClose method. Under normal circumstances, it does. However, if the user uses a ReverseProxy.ModifyResponse hook to replace the Response.Body (for example, a hook might wrap the Body), and the new body does not have a WriteClose method, ReverseProxy will now fail to propagate the close signal at all.

A distressingly complicated test demonstrating the problem:

func TestReverseProxyUpgradeNoCloseWrite(t *testing.T) {
        // The backend hijacks the connection,
        // reads all data from the client,
        // and returns.
        backendDone := make(chan struct{})
        backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
                w.Header().Set("Connection", "upgrade")
                w.Header().Set("Upgrade", "u")
                w.WriteHeader(101)
                conn, _, err := http.NewResponseController(w).Hijack()
                if err != nil {
                        t.Errorf("Hijack: %v", err)
                }
                io.Copy(io.Discard, conn)
                close(backendDone)
        }))
        backendURL, err := url.Parse(backend.URL)
        if err != nil {
                t.Fatal(err)
        }

        // The proxy includes a ModifyResponse function which replaces the response body
        // with its own wrapper, dropping the original body's CloseWrite method.
        proxyHandler := NewSingleHostReverseProxy(backendURL)
        proxyHandler.ModifyResponse = func(resp *http.Response) error {
                type readWriteCloserOnly struct {
                        io.ReadWriteCloser
                }
                resp.Body = readWriteCloserOnly{resp.Body.(io.ReadWriteCloser)}
                return nil
        }
        frontend := httptest.NewServer(proxyHandler)
        defer frontend.Close()

        // The client sends a request and closes the connection.
        req, _ := http.NewRequest("GET", frontend.URL, nil)
        req.Header.Set("Connection", "upgrade")
        req.Header.Set("Upgrade", "u")
        resp, err := frontend.Client().Do(req)
        if err != nil {
                t.Fatal(err)
        }
        resp.Body.Close()

        // We expect that the client's closure of the connection is propagated to the backend.
        <-backendDone
}

[gopherbot]

Change https://go.dev/cl/655595 mentions this issue: net/http/httputil: close hijacked connections when CloseWrite not available

[gopherbot]

Change https://go.dev/cl/655516 mentions this issue: net/http/httputil: close hijacked connections when CloseWrite not available