Improve error message with invalid headers (#2068)

Author: JamesNK

src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs

@@ -1,4 +1,4 @@
-#region Copyright notice and license
+#region Copyright notice and license
 
 // Copyright 2019 The gRPC Authors
 //
@@ -355,21 +355,28 @@ protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders)
             throw new InvalidOperationException("Response headers can only be sent once per call.");
         }
 
-        foreach (var entry in responseHeaders)
+        foreach (var header in responseHeaders)
         {
-            if (entry.Key == GrpcProtocolConstants.CompressionRequestAlgorithmHeader)
+            if (header.Key == GrpcProtocolConstants.CompressionRequestAlgorithmHeader)
             {
                 // grpc-internal-encoding-request is used in the server to set message compression
                 // on a per-call bassis.
                 // 'grpc-encoding' is sent even if WriteOptions.Flags = NoCompress. In that situation
                 // individual messages will not be written with compression.
-                ResponseGrpcEncoding = entry.Value;
+                ResponseGrpcEncoding = header.Value;
                 HttpContext.Response.Headers[GrpcProtocolConstants.MessageEncodingHeader] = ResponseGrpcEncoding;
             }
             else
             {
-                var encodedValue = entry.IsBinary ? Convert.ToBase64String(entry.ValueBytes) : entry.Value;
-                HttpContext.Response.Headers.Append(entry.Key, encodedValue);
+                var encodedValue = header.IsBinary ? Convert.ToBase64String(header.ValueBytes) : header.Value;
+                try
+                {
+                    HttpContext.Response.Headers.Append(header.Key, encodedValue);
+                }
+                catch (Exception ex)
+                {
+                    throw new InvalidOperationException($"Error adding response header '{header.Key}'.", ex);
+                }
             }
         }
 

src/Grpc.AspNetCore.Server/Internal/HttpResponseExtensions.cs

@@ -1,4 +1,4 @@
-#region Copyright notice and license
+#region Copyright notice and license
 
 // Copyright 2019 The gRPC Authors
 //
@@ -31,7 +31,14 @@ public static void ConsolidateTrailers(this HttpResponse httpResponse, HttpConte
             foreach (var trailer in context.ResponseTrailers)
             {
                 var value = (trailer.IsBinary) ? Convert.ToBase64String(trailer.ValueBytes) : trailer.Value;
-                trailersDestination.Append(trailer.Key, value);
+                try
+                {
+                    trailersDestination.Append(trailer.Key, value);
+                }
+                catch (Exception ex)
+                {
+                    throw new InvalidOperationException($"Error adding response trailer '{trailer.Key}'.", ex);
+                }
             }
         }
 

test/FunctionalTests/Client/TrailerMetadataTests.cs test/FunctionalTests/Client/MetadataTests.cs

@@ -1,4 +1,4 @@
-#region Copyright notice and license
+#region Copyright notice and license
 
 // Copyright 2019 The gRPC Authors
 //
@@ -26,7 +26,7 @@
 namespace Grpc.AspNetCore.FunctionalTests.Server;
 
 [TestFixture]
-public class TrailerMetadataTests : FunctionalTestBase
+public class MetadataTests : FunctionalTestBase
 {
     [Test]
     public async Task GetTrailers_UnaryMethodSetStatusWithTrailers_TrailersAvailableInClient()
@@ -224,4 +224,67 @@ async Task UnaryDeadlineExceeded(HelloRequest request, IAsyncStreamWriter<HelloR
 
         AssertHasLogRpcConnectionError(StatusCode.InvalidArgument, "Validation failed");
     }
+
+    [Test]
+    public async Task ServerTrailers_UnaryMethodThrowsExceptionWithInvalidTrailers_FriendlyServerError()
+    {
+        Task<HelloReply> UnaryCall(HelloRequest request, ServerCallContext context)
+        {
+            var trailers = new Metadata();
+            trailers.Add(new Metadata.Entry("Name", "This is invalid: \u0011"));
+            return Task.FromException<HelloReply>(new RpcException(new Status(StatusCode.InvalidArgument, "Validation failed"), trailers));
+        }
+
+        // Arrange
+        SetExpectedErrorsFilter(writeContext => true);
+
+        var method = Fixture.DynamicGrpc.AddUnaryMethod<HelloRequest, HelloReply>(UnaryCall);
+
+        var channel = CreateChannel();
+
+        var client = TestClientFactory.Create(channel, method);
+
+        // Act
+        var call = client.UnaryCall(new HelloRequest());
+
+        var ex = await ExceptionAssert.ThrowsAsync<RpcException>(() => call.ResponseAsync).DefaultTimeout();
+
+        // Assert
+        Assert.AreEqual(StatusCode.Unknown, ex.StatusCode);
+        Assert.AreEqual("Bad gRPC response. HTTP status code: 500", ex.Status.Detail);
+
+        HasLogException(ex => ex.Message == "Error adding response trailer 'name'." && ex.InnerException!.Message == "Invalid non-ASCII or control character in header: 0x0011");
+    }
+
+    [Test]
+    public async Task ServerHeaders_UnaryMethodThrowsExceptionWithInvalidTrailers_FriendlyServerError()
+    {
+        async Task<HelloReply> UnaryCall(HelloRequest request, ServerCallContext context)
+        {
+            var headers = new Metadata();
+            headers.Add(new Metadata.Entry("Name", "This is invalid: \u0011"));
+            await context.WriteResponseHeadersAsync(headers);
+            return new HelloReply();
+        }
+
+        // Arrange
+        SetExpectedErrorsFilter(writeContext => true);
+
+        var method = Fixture.DynamicGrpc.AddUnaryMethod<HelloRequest, HelloReply>(UnaryCall);
+
+        var channel = CreateChannel();
+
+        var client = TestClientFactory.Create(channel, method);
+
+        // Act
+        var call = client.UnaryCall(new HelloRequest());
+
+        var ex = await ExceptionAssert.ThrowsAsync<RpcException>(() => call.ResponseAsync).DefaultTimeout();
+
+        // Assert
+        Assert.AreEqual(StatusCode.Unknown, ex.StatusCode);
+        Assert.AreEqual("Exception was thrown by handler. InvalidOperationException: Error adding response header 'name'. InvalidOperationException: Invalid non-ASCII or control character in header: 0x0011", ex.Status.Detail);
+
+        HasLogException(ex => ex.Message == "Error adding response header 'name'." && ex.InnerException!.Message == "Invalid non-ASCII or control character in header: 0x0011");
+    }
 }