Restrict metadata headers in error propagation

connect_ext_test.go

@@ -43,6 +43,7 @@ import (
 	"connectrpc.com/connect/internal/memhttp/memhttptest"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/reflect/protoregistry"
+	"google.golang.org/protobuf/types/known/wrapperspb"
 )
 
 const errorMessage = "oh no"
@@ -542,6 +543,82 @@ func TestConcurrentStreams(t *testing.T) {
 	done.Wait()
 }
 
+func TestErrorHeaderPropagation(t *testing.T) {
+	t.Parallel()
+	pingServer := &pluggablePingServer{
+		ping: func(ctx context.Context, request *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error) {
+			err := connect.NewError(connect.CodeInvalidArgument, errors.New("ping error"))
+			msgDetail := &wrapperspb.StringValue{Value: "server details"}
+			errDetail, derr := connect.NewErrorDetail(msgDetail)
+			if derr != nil {
+				return nil, derr
+			}
+			err.AddDetail(errDetail)
+			err.Meta().Set("Content-Length", "1337")
+			err.Meta().Set("Content-Type", "application/xml")
+			err.Meta().Set("Accept-Encoding", "bogus")
+			err.Meta().Set("Date", "Thu, 01 Jan 1970 00:00:00 GMT")
+			err.Meta().Set("X-Test", "test") // Allow custom headers.
+			return nil, err
+		},
+	}
+	mux := http.NewServeMux()
+	mux.Handle(pingv1connect.NewPingServiceHandler(pingServer))
+	server := memhttptest.NewServer(t, mux)
+
+	testPing := func(t *testing.T, client pingv1connect.PingServiceClient) {
+		t.Helper()
+		request := connect.NewRequest(&pingv1.PingRequest{})
+		request.Header().Set("X-Test", t.Name())
+		_, err := client.Ping(context.Background(), request)
+		if !assert.NotNil(t, err) {
+			return
+		}
+		assert.Equal(t, connect.CodeOf(err), connect.CodeInvalidArgument)
+		var connectErr *connect.Error
+		if !assert.True(t, errors.As(err, &connectErr)) {
+			return
+		}
+		assert.Equal(t, connectErr.Message(), "ping error")
+
+		details := connectErr.Details()
+		if assert.Equal(t, len(details), 1) {
+			detailMsg, err := details[0].Value()
+			if !assert.Nil(t, err) {
+				return
+			}
+			serverDetails, ok := detailMsg.(*wrapperspb.StringValue)
+			if !assert.True(t, ok) {
+				return
+			}
+			assert.Equal(t, serverDetails.Value, "server details")
+		}
+		meta := connectErr.Meta()
+		assert.NotEqual(t, meta.Values("Content-Length"), []string{"1337"})
+		assert.NotEqual(t, meta.Values("Accept-Encoding"), []string{"bogus"})
+		assert.NotEqual(t, meta.Values("Content-Type"), []string{"application/xml"})
+		assert.NotEqual(t, meta.Values("Content-Length"), []string{"1337"})
+		assert.NotEqual(t, meta.Values("Date"), []string{"Thu, 01 Jan 1970 00:00:00 GMT"})
+		assert.Equal(t, meta.Values("X-Test"), []string{"test"})
+	}
+
+	t.Run("connect", func(t *testing.T) {
+		t.Parallel()
+		client := pingv1connect.NewPingServiceClient(server.Client(), server.URL())
+		testPing(t, client)
+	})
+	t.Run("grpc", func(t *testing.T) {
+		t.Parallel()
+		client := pingv1connect.NewPingServiceClient(server.Client(), server.URL(), connect.WithGRPC())
+		testPing(t, client)
+	})
+	t.Run("grpc-web", func(t *testing.T) {
+		t.Parallel()
+		client := pingv1connect.NewPingServiceClient(server.Client(), server.URL(), connect.WithGRPCWeb())
+		testPing(t, client)
+	})
+}
+
 func TestHeaderBasic(t *testing.T) {
 	t.Parallel()
 	const (

header.go

@@ -57,6 +57,46 @@ func mergeHeaders(into, from http.Header) {
 	}
 }
 
+// mergeMetdataHeaders merges the metadata headers from the "from" header into
+// the "into" header. It skips over non metadata headers that should not be
+// propagated from the server to the client.
+func mergeMetadataHeaders(into, from http.Header) {
+	for key, vals := range from {
+		if len(vals) == 0 {
+			// For response trailers, net/http will pre-populate entries
+			// with nil values based on the "Trailer" header. But if there
+			// are no actual values for those keys, we skip them.
+			continue
+		}
+		switch http.CanonicalHeaderKey(key) {
+		case headerContentType,
+			headerContentLength,
+			headerContentEncoding,
+			headerHost,
+			headerUserAgent,
+			headerTrailer,
+			headerDate:
+			// HTTP headers.
+		case connectUnaryHeaderAcceptCompression,
+			connectUnaryTrailerPrefix,
+			connectStreamingHeaderCompression,
+			connectStreamingHeaderAcceptCompression,
+			connectHeaderTimeout,
+			connectHeaderProtocolVersion:
+			// Connect headers.
+		case grpcHeaderCompression,
+			grpcHeaderAcceptCompression,
+			grpcHeaderTimeout,
+			grpcHeaderStatus,
+			grpcHeaderMessage,
+			grpcHeaderDetails:
+			// gRPC headers.
+		default:
+			into[key] = append(into[key], vals...)
+		}
+	}
+}
+
 // getHeaderCanonical is a shortcut for Header.Get() which
 // bypasses the CanonicalMIMEHeaderKey operation when we
 // know the key is already in canonical form.

protocol.go

@@ -41,6 +41,7 @@ const (
 	headerHost            = "Host"
 	headerUserAgent       = "User-Agent"
 	headerTrailer         = "Trailer"
+	headerDate            = "Date"
 
 	discardLimit = 1024 * 1024 * 4 // 4MiB
 )

protocol_connect.go

@@ -534,10 +534,12 @@ func (cc *connectUnaryClientConn) validateResponse(response *http.Response) *Err
 			cc.compressionPools.CommaSeparatedNames(),
 		)
 	}
+	cc.unmarshaler.compressionPool = cc.compressionPools.Get(compression)
 	if response.StatusCode != http.StatusOK {
 		unmarshaler := connectUnaryUnmarshaler{
+			ctx:             cc.unmarshaler.ctx,
 			reader:          response.Body,
-			compressionPool: cc.compressionPools.Get(compression),
+			compressionPool: cc.unmarshaler.compressionPool,
 			bufferPool:      cc.bufferPool,
 		}
 		var wireErr connectWireError
@@ -559,7 +561,6 @@ func (cc *connectUnaryClientConn) validateResponse(response *http.Response) *Err
 		mergeHeaders(serverErr.meta, cc.responseTrailer)
 		return serverErr
 	}
-	cc.unmarshaler.compressionPool = cc.compressionPools.Get(compression)
 	return nil
 }
 
@@ -766,7 +767,7 @@ func (hc *connectUnaryHandlerConn) writeResponseHeader(err error) {
 	}
 	if err != nil {
 		if connectErr, ok := asError(err); ok {
-			mergeHeaders(header, connectErr.meta)
+			mergeMetadataHeaders(header, connectErr.meta)
 		}
 	}
 	for k, v := range hc.responseTrailer {

protocol_grpc.go

@@ -860,7 +860,7 @@ func grpcErrorToTrailer(trailer http.Header, protobuf Codec, err error) {
 		return
 	}
 	if connectErr, ok := asError(err); ok {
-		mergeHeaders(trailer, connectErr.meta)
+		mergeMetadataHeaders(trailer, connectErr.meta)
 	}
 	setHeaderCanonical(trailer, grpcHeaderStatus, code)
 	setHeaderCanonical(trailer, grpcHeaderMessage, grpcPercentEncode(status.GetMessage()))