From a9970e7848974e95a2151eb3d78d38857c8673e6 Mon Sep 17 00:00:00 2001 From: Rajesh Rajendiran Date: Sun, 2 Aug 2026 21:16:08 +0000 Subject: [PATCH 1/2] fix: return 400 instead of 502 for a Host header with a non-numeric port connectRequest.Dest can carry the eyeball's Host header verbatim, so a port like ":aaaa" fails Go's URL parser inside http.NewRequestWithContext. That error was surfacing to the eyeball as a 502, which looks like an origin failure rather than a malformed client request. Wrap it in a distinguishable error type so handleDataStream can report it as 400. --- connection/quic_connection.go | 16 +++++- connection/quic_connection_test.go | 78 ++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/connection/quic_connection.go b/connection/quic_connection.go index 6133a022fbc..36220e0af69 100644 --- a/connection/quic_connection.go +++ b/connection/quic_connection.go @@ -39,6 +39,16 @@ const ( QUICMetadataFlowID = "FlowID" ) +// malformedRequestError indicates the request could not be built because of client-supplied data, such as a +// Host header with a non-numeric port. It is reported to the eyeball as a 400 rather than a 502, since the +// failure is not caused by the origin or the connection to it. +type malformedRequestError struct { + cause error +} + +func (e *malformedRequestError) Error() string { return e.cause.Error() } +func (e *malformedRequestError) Unwrap() error { return e.cause } + // quicConnection represents the type that facilitates Proxying via QUIC streams. type quicConnection struct { conn cfdquic.QUICConnection @@ -206,6 +216,10 @@ func (q *quicConnection) handleDataStream(ctx context.Context, stream *rpcquic.R if errors.Is(err, cfdflow.ErrTooManyActiveFlows) { metadata = append(metadata, pogs.ErrorFlowConnectRateLimitedMetadata) } + var malformedErr *malformedRequestError + if errors.As(err, &malformedErr) { + metadata = append(metadata, pogs.Metadata{Key: HTTPStatus, Val: strconv.Itoa(http.StatusBadRequest)}) + } if writeRespErr := stream.WriteConnectResponseData(err, metadata...); writeRespErr != nil { return writeRespErr @@ -352,7 +366,7 @@ func buildHTTPRequest( req, err := http.NewRequestWithContext(ctx, method, dest, body) if err != nil { - return nil, err + return nil, &malformedRequestError{cause: err} } req.Host = host diff --git a/connection/quic_connection_test.go b/connection/quic_connection_test.go index 2df741a6a9e..c0c960adfbe 100644 --- a/connection/quic_connection_test.go +++ b/connection/quic_connection_test.go @@ -16,6 +16,7 @@ import ( "net/http" "net/netip" "net/url" + "strconv" "strings" "testing" "time" @@ -509,6 +510,26 @@ func TestBuildHTTPRequest(t *testing.T) { } } +// TestBuildHTTPRequestMalformedDest verifies that a Dest containing a non-numeric port (e.g. forwarded +// verbatim from a client's malformed Host header) is reported as malformedRequestError, not a bare error, +// so that callers can distinguish a bad request from an origin/proxy failure. +func TestBuildHTTPRequestMalformedDest(t *testing.T) { + log := zerolog.Nop() + connectRequest := &pogs.ConnectRequest{ + Dest: "http://test.com:aaaa/foo", + Metadata: []pogs.Metadata{ + {Key: "HttpHost", Val: "test.com:aaaa"}, + {Key: "HttpMethod", Val: "get"}, + }, + } + + _, err := buildHTTPRequest(t.Context(), connectRequest, io.NopCloser(&bytes.Buffer{}), 0, &log) + require.Error(t, err) + + var malformedErr *malformedRequestError + require.ErrorAs(t, err, &malformedErr) +} + func (moc *mockOriginProxyWithRequest) ProxyTCP(ctx context.Context, rwa ReadWriteAcker, tcpRequest *TCPRequest) error { if tcpRequest.Dest == "rate-limit-me" { return pkgerrors.Wrap(cfdflow.ErrTooManyActiveFlows, "failed tcp stream") @@ -658,6 +679,63 @@ func TestTCPProxy_FlowRateLimited(t *testing.T) { <-connDone } +// TestHTTPProxy_MalformedHostPort tests that a Dest with a non-numeric port (as would be forwarded from a +// client's malformed Host header) results in a 400 response to the eyeball instead of the default 502. +func TestHTTPProxy_MalformedHostPort(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + + // Start a UDP Listener for QUIC. + udpAddr, err := net.ResolveUDPAddr("udp", "127.0.0.1:0") + require.NoError(t, err) + + udpListener, err := net.ListenUDP(udpAddr.Network(), udpAddr) + require.NoError(t, err) + defer func() { _ = udpListener.Close() }() + + quicTransport := &quic.Transport{Conn: udpListener, ConnectionIDLength: 16} + quicListener, err := quicTransport.Listen(testTLSServerConfig, testQUICConfig) + require.NoError(t, err) + + serverDone := make(chan struct{}) + go func() { + defer close(serverDone) + + session, err := quicListener.Accept(ctx) + assert.NoError(t, err) + + quicStream, err := session.OpenStreamSync(t.Context()) + assert.NoError(t, err) + stream := cfdquic.NewSafeStreamCloser(quicStream, defaultQUICTimeout, &log) + + reqClientStream := rpcquic.RequestClientStream{ReadWriteCloser: stream} + err = reqClientStream.WriteConnectRequestData( + "http://test.com:aaaa/foo", + pogs.ConnectionTypeHTTP, + pogs.Metadata{Key: HTTPHostKey, Val: "test.com:aaaa"}, + pogs.Metadata{Key: HTTPMethodKey, Val: "GET"}, + ) + assert.NoError(t, err) + + response, err := reqClientStream.ReadConnectResponseData() + assert.NoError(t, err) + + assert.NotEmpty(t, response.Error) + assert.Contains(t, response.Metadata, pogs.Metadata{Key: HTTPStatus, Val: strconv.Itoa(http.StatusBadRequest)}) + }() + + tunnelConn, _ := testTunnelConnection(t, netip.MustParseAddrPort(udpListener.LocalAddr().String()), uint8(0)) + + connDone := make(chan struct{}) + go func() { + defer close(connDone) + _ = tunnelConn.Serve(ctx) + }() + + <-serverDone + cancel() + <-connDone +} + func testCreateUDPConnReuseSourcePortForEdgeIP(t *testing.T, edgeIP netip.AddrPort) { logger := zerolog.Nop() conn, err := createUDPConnForConnIndex(0, nil, edgeIP, dialopts.DialOpts{}, &logger) From af87ce036c544782a3b8812aa503608fc2889b0a Mon Sep 17 00:00:00 2001 From: Rajesh Rajendiran Date: Sun, 2 Aug 2026 21:20:52 +0000 Subject: [PATCH 2/2] fix: use sentinel error instead of custom type for malformed request errors.Is against a sentinel matches the existing cfdflow.ErrTooManyActiveFlows check right above it in handleDataStream, and needs less code than a custom struct type with hand-written Error()/Unwrap() methods. --- connection/quic_connection.go | 17 +++++------------ connection/quic_connection_test.go | 7 ++----- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/connection/quic_connection.go b/connection/quic_connection.go index 36220e0af69..e1b07ea9618 100644 --- a/connection/quic_connection.go +++ b/connection/quic_connection.go @@ -39,15 +39,9 @@ const ( QUICMetadataFlowID = "FlowID" ) -// malformedRequestError indicates the request could not be built because of client-supplied data, such as a -// Host header with a non-numeric port. It is reported to the eyeball as a 400 rather than a 502, since the -// failure is not caused by the origin or the connection to it. -type malformedRequestError struct { - cause error -} - -func (e *malformedRequestError) Error() string { return e.cause.Error() } -func (e *malformedRequestError) Unwrap() error { return e.cause } +// errMalformedRequest marks a buildHTTPRequest failure caused by client-supplied data, such as a Host +// header with a non-numeric port, so it is reported to the eyeball as 400 instead of the default 502. +var errMalformedRequest = errors.New("malformed request") // quicConnection represents the type that facilitates Proxying via QUIC streams. type quicConnection struct { @@ -216,8 +210,7 @@ func (q *quicConnection) handleDataStream(ctx context.Context, stream *rpcquic.R if errors.Is(err, cfdflow.ErrTooManyActiveFlows) { metadata = append(metadata, pogs.ErrorFlowConnectRateLimitedMetadata) } - var malformedErr *malformedRequestError - if errors.As(err, &malformedErr) { + if errors.Is(err, errMalformedRequest) { metadata = append(metadata, pogs.Metadata{Key: HTTPStatus, Val: strconv.Itoa(http.StatusBadRequest)}) } @@ -366,7 +359,7 @@ func buildHTTPRequest( req, err := http.NewRequestWithContext(ctx, method, dest, body) if err != nil { - return nil, &malformedRequestError{cause: err} + return nil, fmt.Errorf("%w: %s", errMalformedRequest, err) } req.Host = host diff --git a/connection/quic_connection_test.go b/connection/quic_connection_test.go index c0c960adfbe..5f57e1d868a 100644 --- a/connection/quic_connection_test.go +++ b/connection/quic_connection_test.go @@ -511,7 +511,7 @@ func TestBuildHTTPRequest(t *testing.T) { } // TestBuildHTTPRequestMalformedDest verifies that a Dest containing a non-numeric port (e.g. forwarded -// verbatim from a client's malformed Host header) is reported as malformedRequestError, not a bare error, +// verbatim from a client's malformed Host header) is reported as errMalformedRequest, not a bare error, // so that callers can distinguish a bad request from an origin/proxy failure. func TestBuildHTTPRequestMalformedDest(t *testing.T) { log := zerolog.Nop() @@ -524,10 +524,7 @@ func TestBuildHTTPRequestMalformedDest(t *testing.T) { } _, err := buildHTTPRequest(t.Context(), connectRequest, io.NopCloser(&bytes.Buffer{}), 0, &log) - require.Error(t, err) - - var malformedErr *malformedRequestError - require.ErrorAs(t, err, &malformedErr) + require.ErrorIs(t, err, errMalformedRequest) } func (moc *mockOriginProxyWithRequest) ProxyTCP(ctx context.Context, rwa ReadWriteAcker, tcpRequest *TCPRequest) error {