Merge commit from fork

* fix(reverseproxy): hop-by-hop header fix

* test(headers): Added unit tests for hop-by-hope header behavior

modules/caddyhttp/reverseproxy/hopheaders_test.go

@@ -0,0 +1,191 @@
+package reverseproxy
+
+import (
+	"context"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/caddyserver/caddy/v2"
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
+)
+
+// 101 responses strip hop-by-hop headers (Alt-Svc, Keep-Alive, etc.) but preserve Upgrade and Connection
+func TestFinalizeResponse_101_StripsHopByHopHeaders(t *testing.T) {
+	h := &Handler{logger: caddy.Log()}
+
+	req := httptest.NewRequest(http.MethodGet, "/ws", nil)
+	req.Header.Set("Upgrade", "websocket")
+	req.Header.Set("Connection", "Upgrade")
+	req.Header.Set("Sec-WebSocket-Version", "13")
+	req.Header.Set("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==")
+
+	vars := map[string]any{}
+	ctx := context.WithValue(req.Context(), caddyhttp.VarsCtxKey, vars)
+	req = req.WithContext(ctx)
+
+	res := &http.Response{
+		StatusCode: http.StatusSwitchingProtocols,
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header: http.Header{
+			"Upgrade":              {"websocket"},
+			"Connection":           {"Upgrade"},
+			"Sec-Websocket-Accept": {"s3pPLMBiTxaQ9kYGzzhZRbK+xOo="},
+			"Alt-Svc":              {"h2=\"evil.com:443\"; ma=86400"},
+			"Keep-Alive":           {"timeout=999"},
+			"Proxy-Authenticate":   {"Basic realm=\"phish\""},
+		},
+		Body: fakeRWC{strings.NewReader("")},
+	}
+
+	repl := caddy.NewReplacer()
+	rw := httptest.NewRecorder()
+
+	err := h.finalizeResponse(rw, req, res, repl, fakeStart, caddy.Log())
+	if err != nil {
+		t.Logf("finalizeResponse returned error (expected, no real conn): %v", err)
+	}
+
+	if got := rw.Header().Get("Upgrade"); got != "websocket" {
+		t.Errorf("Upgrade = %q, want %q", got, "websocket")
+	}
+	if got := rw.Header().Get("Connection"); got != "Upgrade" {
+		t.Errorf("Connection = %q, want %q", got, "Upgrade")
+	}
+	for _, hdr := range []string{"Alt-Svc", "Keep-Alive", "Proxy-Authenticate"} {
+		if got := rw.Header().Get(hdr); got != "" {
+			t.Errorf("%s = %q, want empty (should be stripped)", hdr, got)
+		}
+	}
+}
+
+// Headers named in the upstream Connection value are stripped
+func TestFinalizeResponse_101_StripsConnectionNamedHeaders(t *testing.T) {
+	h := &Handler{logger: caddy.Log()}
+
+	req := httptest.NewRequest(http.MethodGet, "/ws", nil)
+	req.Header.Set("Upgrade", "websocket")
+	req.Header.Set("Connection", "Upgrade")
+	req.Header.Set("Sec-WebSocket-Version", "13")
+	req.Header.Set("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==")
+
+	vars := map[string]any{}
+	ctx := context.WithValue(req.Context(), caddyhttp.VarsCtxKey, vars)
+	req = req.WithContext(ctx)
+
+	res := &http.Response{
+		StatusCode: http.StatusSwitchingProtocols,
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header: http.Header{
+			"Upgrade":              {"websocket"},
+			"Connection":           {"Upgrade, X-Custom-ID"},
+			"Sec-Websocket-Accept": {"s3pPLMBiTxaQ9kYGzzhZRbK+xOo="},
+			"X-Custom-Id":          {"should-be-stripped"},
+		},
+		Body: fakeRWC{strings.NewReader("")},
+	}
+
+	repl := caddy.NewReplacer()
+	rw := httptest.NewRecorder()
+
+	err := h.finalizeResponse(rw, req, res, repl, fakeStart, caddy.Log())
+	if err != nil {
+		t.Logf("finalizeResponse returned error (no real connection): %v", err)
+	}
+
+	if got := rw.Header().Get("Upgrade"); got != "websocket" {
+		t.Errorf("Upgrade = %q, want %q", got, "websocket")
+	}
+	if got := rw.Header().Get("X-Custom-Id"); got != "" {
+		t.Errorf("X-Custom-Id = %q, want empty (named in Connection, should be stripped)", got)
+	}
+	if got := rw.Header().Get("Connection"); got != "Upgrade" {
+		t.Errorf("Connection = %q, want %q", got, "Upgrade")
+	}
+}
+
+// Normal 200 responses strip hop-by-hop headers
+func TestFinalizeResponse_200_StillStripsHopByHop(t *testing.T) {
+	h := &Handler{logger: caddy.Log()}
+
+	req := httptest.NewRequest(http.MethodGet, "/", nil)
+	vars := map[string]any{}
+	ctx := context.WithValue(req.Context(), caddyhttp.VarsCtxKey, vars)
+	req = req.WithContext(ctx)
+
+	res := &http.Response{
+		StatusCode: http.StatusOK,
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header: http.Header{
+			"Content-Type": {"text/plain"},
+			"Alt-Svc":      {"h2=\"evil.com:443\""},
+			"Keep-Alive":   {"timeout=999"},
+		},
+		Body: io.NopCloser(strings.NewReader("ok")),
+	}
+
+	repl := caddy.NewReplacer()
+	rw := httptest.NewRecorder()
+
+	err := h.finalizeResponse(rw, req, res, repl, fakeStart, caddy.Log())
+	if err != nil {
+		t.Fatalf("finalizeResponse returned error: %v", err)
+	}
+
+	for _, hdr := range []string{"Alt-Svc", "Keep-Alive"} {
+		if got := rw.Header().Get(hdr); got != "" {
+			t.Errorf("%s = %q on 200 response, want empty (stripped)", hdr, got)
+		}
+	}
+}
+
+// Stripping still runs when the client didn't request an upgrade
+func TestFinalizeResponse_101_NoUpgradeRequest(t *testing.T) {
+	h := &Handler{logger: caddy.Log()}
+
+	req := httptest.NewRequest(http.MethodGet, "/ws", nil)
+
+	vars := map[string]any{}
+	ctx := context.WithValue(req.Context(), caddyhttp.VarsCtxKey, vars)
+	req = req.WithContext(ctx)
+
+	res := &http.Response{
+		StatusCode: http.StatusSwitchingProtocols,
+		ProtoMajor: 1,
+		ProtoMinor: 1,
+		Header: http.Header{
+			"Upgrade":    {"websocket"},
+			"Connection": {"Upgrade"},
+			"Alt-Svc":    {"h2=\"evil.com:443\""},
+		},
+		Body: io.NopCloser(strings.NewReader("")),
+	}
+
+	repl := caddy.NewReplacer()
+	rw := httptest.NewRecorder()
+
+	err := h.finalizeResponse(rw, req, res, repl, fakeStart, caddy.Log())
+	if err != nil {
+		t.Fatalf("finalizeResponse returned error: %v", err)
+	}
+
+	if got := rw.Header().Get("Alt-Svc"); got != "" {
+		t.Errorf("Alt-Svc = %q, want empty (stripped)", got)
+	}
+}
+
+// fakeRWC lets handleUpgradeResponse proceed past the body type assertion in tests
+type fakeRWC struct {
+	io.Reader
+}
+
+func (f fakeRWC) Write(p []byte) (n int, err error) { return len(p), nil }
+func (f fakeRWC) Close() error                      { return nil }
+
+var fakeStart = time.Time{}

modules/caddyhttp/reverseproxy/reverseproxy.go

@@ -1215,6 +1215,22 @@ func (h *Handler) finalizeResponse(
 	start time.Time,
 	logger *zap.Logger,
 ) error {
+	// Strip hop-by-hop headers from the upstream response.
+	// For 101 Switching Protocols, save the Upgrade value
+	// first (handleUpgradeResponse needs it for validation)
+	// and restore it with a canonical Connection header.
+	upgradeVal := res.Header.Get("Upgrade")
+
+	removeConnectionHeaders(res.Header)
+	for _, h := range hopHeaders {
+		res.Header.Del(h)
+	}
+
+	if res.StatusCode == http.StatusSwitchingProtocols && upgradeVal != "" {
+		res.Header.Set("Upgrade", upgradeVal)
+		res.Header.Set("Connection", "Upgrade")
+	}
+
 	// deal with 101 Switching Protocols responses: (WebSocket, h2c, etc)
 	if res.StatusCode == http.StatusSwitchingProtocols {
 		var wg sync.WaitGroup
@@ -1223,12 +1239,6 @@ func (h *Handler) finalizeResponse(
 		return nil
 	}
 
-	removeConnectionHeaders(res.Header)
-
-	for _, h := range hopHeaders {
-		res.Header.Del(h)
-	}
-
 	// delete our Server header and use Via instead (see #6275)
 	rw.Header().Del("Server")
 	var protoPrefix string