fix: return 400 instead of 502 for a Host header with a non-numeric port

connection/quic_connection.go

@@ -39,6 +39,10 @@ const (
 	QUICMetadataFlowID = "FlowID"
 )
 
+// 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 {
 	conn                 cfdquic.QUICConnection
@@ -206,6 +210,9 @@ func (q *quicConnection) handleDataStream(ctx context.Context, stream *rpcquic.R
 		if errors.Is(err, cfdflow.ErrTooManyActiveFlows) {
 			metadata = append(metadata, pogs.ErrorFlowConnectRateLimitedMetadata)
 		}
+		if errors.Is(err, errMalformedRequest) {
+			metadata = append(metadata, pogs.Metadata{Key: HTTPStatus, Val: strconv.Itoa(http.StatusBadRequest)})
+		}
 
 		if writeRespErr := stream.WriteConnectResponseData(err, metadata...); writeRespErr != nil {
 			return writeRespErr
@@ -352,7 +359,7 @@ func buildHTTPRequest(
 
 	req, err := http.NewRequestWithContext(ctx, method, dest, body)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("%w: %s", errMalformedRequest, err)
 	}
 
 	req.Host = host

connection/quic_connection_test.go

@@ -16,6 +16,7 @@ import (
 	"net/http"
 	"net/netip"
 	"net/url"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -509,6 +510,23 @@ 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 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()
+	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.ErrorIs(t, err, errMalformedRequest)
+}
+
 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 +676,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)