From 408432fa20afbf0689a872b239a2479f64a1ee4e Mon Sep 17 00:00:00 2001 From: Andriy Palamarchuk Date: Sat, 3 Oct 2026 20:11:45 -0400 Subject: [PATCH] fix(proxy): return 502 when forwarding to upstream fails When an allowed request could not be forwarded (for example, DNS failure or connection refused), forwardRequest logged the error and returned without writing a response. Transparent and explicit HTTP clients saw an empty reply. Inside a CONNECT tunnel the client waited for a response while the proxy waited for the next request, so the request hung until the client timed out. Write a 502 Bad Gateway with the reason whenever forwarding fails before a response has been written. Co-Authored-By: Claude Opus 5.5 (1M context) --- proxy/proxy.go | 28 ++++++++++ proxy/proxy_upstream_error_test.go | 89 ++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 proxy/proxy_upstream_error_test.go diff --git a/proxy/proxy.go b/proxy/proxy.go index d3ec4c6..58de994 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -380,6 +380,7 @@ func (p *Server) forwardRequest(conn net.Conn, req *http.Request, https bool, se newReq, err := http.NewRequest(req.Method, targetURL.String(), body) if err != nil { p.logger.Error("can't create http request", "error", err) + p.writeBadGatewayResponse(conn, req, err) return } @@ -403,6 +404,7 @@ func (p *Server) forwardRequest(conn net.Conn, req *http.Request, https bool, se resp, err := client.Do(newReq) if err != nil { p.logger.Error("Failed to forward HTTPS request", "error", err) + p.writeBadGatewayResponse(conn, req, err) return } @@ -418,6 +420,8 @@ func (p *Server) forwardRequest(conn net.Conn, req *http.Request, https bool, se bodyBytes, err := io.ReadAll(resp.Body) if err != nil { p.logger.Error("can't read response body", "error", err) + _ = resp.Body.Close() + p.writeBadGatewayResponse(conn, req, err) return } resp.Header.Add("Content-Length", strconv.Itoa(len(bodyBytes))) @@ -503,6 +507,30 @@ For more help: https://github.com/coder/boundary p.logger.Debug("Successfully wrote to connection") } +// writeBadGatewayResponse tells the client that an allowed request could not +// be forwarded. Every request needs a response: in a CONNECT tunnel the +// client would otherwise wait for one while the proxy waits for the next +// request. +func (p *Server) writeBadGatewayResponse(conn net.Conn, req *http.Request, forwardErr error) { + body := fmt.Sprintf("Boundary could not forward the request to %s: %v\n", req.Host, forwardErr) + + resp := &http.Response{ + Status: "502 Bad Gateway", + StatusCode: http.StatusBadGateway, + Proto: "HTTP/1.1", + ProtoMajor: 1, + ProtoMinor: 1, + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(body)), + ContentLength: int64(len(body)), + } + resp.Header.Set("Content-Type", "text/plain") + + if err := resp.Write(conn); err != nil { + p.logger.Error("Failed to write bad gateway response", "error", err) + } +} + // connectionWrapper lets us "unread" the peeked byte type connectionWrapper struct { net.Conn diff --git a/proxy/proxy_upstream_error_test.go b/proxy/proxy_upstream_error_test.go new file mode 100644 index 0000000..d7619aa --- /dev/null +++ b/proxy/proxy_upstream_error_test.go @@ -0,0 +1,89 @@ +package proxy + +import ( + "io" + "net" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// unreachableAddr returns a loopback address that refuses connections. +func unreachableAddr(t *testing.T) string { + t.Helper() + + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := ln.Addr().String() + require.NoError(t, ln.Close()) + return addr +} + +// TestProxyServerUpstreamUnreachable verifies that an allowed request whose +// upstream cannot be reached gets a 502 Bad Gateway instead of an empty +// reply or a hung connection. +func TestProxyServerUpstreamUnreachable(t *testing.T) { + addr := unreachableAddr(t) + + pt := NewProxyTest(t, + WithCertManager(t.TempDir()), + WithAllowedDomain("127.0.0.1"), + ).Start() + defer pt.Stop() + + expectBadGateway := func(t *testing.T, client *http.Client, url, host string) { + t.Helper() + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, url, nil) + require.NoError(t, err) + req.Host = host + + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() //nolint:errcheck + + require.Equal(t, http.StatusBadGateway, resp.StatusCode) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Contains(t, string(body), addr) + } + + t.Run("TransparentHTTP", func(t *testing.T) { + expectBadGateway(t, pt.client, "http://localhost:8080/", addr) + }) + + t.Run("TransparentHTTPS", func(t *testing.T) { + expectBadGateway(t, pt.client, "https://localhost:8080/", addr) + }) + + t.Run("ExplicitProxyHTTP", func(t *testing.T) { + expectBadGateway(t, pt.proxyClient, "http://"+addr+"/", "") + }) + + // The CONNECT tunnel serves many requests on one connection, so a + // failed forward must still produce a response. Otherwise the client + // waits for it while the proxy waits for the next request. + t.Run("CONNECTTunnel", func(t *testing.T) { + // The CONNECT target only sets the TLS server name, which must be a + // hostname. The Host header below picks the upstream. + tunnel, err := pt.establishExplicitCONNECT("localhost:443") + require.NoError(t, err) + defer tunnel.close() //nolint:errcheck + require.NoError(t, tunnel.tlsConn.SetDeadline(time.Now().Add(5*time.Second))) + + // Two requests show that the tunnel stays usable after a failure. + for range 2 { + _, err := tunnel.tlsConn.Write([]byte("GET / HTTP/1.1\r\nHost: " + addr + "\r\n\r\n")) + require.NoError(t, err) + + resp, err := http.ReadResponse(tunnel.reader, nil) + require.NoError(t, err) + _, err = io.Copy(io.Discard, resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusBadGateway, resp.StatusCode) + } + }) +}