From d036704c1068782e70eb030c7c03fa1dccbee87c Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 21 May 2020 23:28:13 +1200 Subject: [PATCH 1/2] Fix sending and recieving duplicate header names --- .../Internal/HttpContextServerCallContext.cs | 4 +- .../Internal/GrpcProtocolHelpers.cs | 16 ++++-- test/FunctionalTests/Client/StreamingTests.cs | 57 +++++++++++++++++++ 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs b/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs index f44c702b9..636e249a8 100644 --- a/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs +++ b/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs @@ -364,11 +364,11 @@ protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) { if (entry.IsBinary) { - HttpContext.Response.Headers[entry.Key] = Convert.ToBase64String(entry.ValueBytes); + HttpContext.Response.Headers.Append(entry.Key, Convert.ToBase64String(entry.ValueBytes)); } else { - HttpContext.Response.Headers[entry.Key] = entry.Value; + HttpContext.Response.Headers.Append(entry.Key, entry.Value); } } } diff --git a/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs b/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs index d0c54d8d3..4900c17e5 100644 --- a/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs +++ b/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs @@ -74,13 +74,17 @@ public static Metadata BuildMetadata(HttpResponseHeaders responseHeaders) { continue; } - else if (header.Key.EndsWith(Metadata.BinaryHeaderSuffix, StringComparison.OrdinalIgnoreCase)) - { - headers.Add(header.Key, GrpcProtocolHelpers.ParseBinaryHeader(string.Join(",", header.Value))); - } - else + + foreach (var value in header.Value) { - headers.Add(header.Key, string.Join(",", header.Value)); + if (header.Key.EndsWith(Metadata.BinaryHeaderSuffix, StringComparison.OrdinalIgnoreCase)) + { + headers.Add(header.Key, ParseBinaryHeader(value)); + } + else + { + headers.Add(header.Key, value); + } } } diff --git a/test/FunctionalTests/Client/StreamingTests.cs b/test/FunctionalTests/Client/StreamingTests.cs index ab55f09d2..441df04b0 100644 --- a/test/FunctionalTests/Client/StreamingTests.cs +++ b/test/FunctionalTests/Client/StreamingTests.cs @@ -458,6 +458,63 @@ async Task ServerStreamingWithTrailers(DataMessage request, IServerStreamWriter< Assert.AreEqual(StatusCode.OK, call.GetStatus().StatusCode); } + [Test] + public async Task ServerStreaming_ThrowErrorWithTrailers_TrailersReturnedToClient() + { + async Task ServerStreamingWithTrailers(DataMessage request, IServerStreamWriter responseStream, ServerCallContext context) + { + await context.WriteResponseHeadersAsync(new Metadata + { + { "Key", "Value1" }, + { "Key", "Value2" }, + }); + await responseStream.WriteAsync(new DataMessage()); + await responseStream.WriteAsync(new DataMessage()); + await responseStream.WriteAsync(new DataMessage()); + await responseStream.WriteAsync(new DataMessage()); + context.ResponseTrailers.Add("Key", "ResponseTrailers"); + throw new RpcException(new Status(StatusCode.Aborted, "Message"), new Metadata + { + { "Key", "RpcException" } + }); + } + + // Arrange + var method = Fixture.DynamicGrpc.AddServerStreamingMethod(ServerStreamingWithTrailers); + + var channel = CreateChannel(); + + var client = TestClientFactory.Create(channel, method); + + // Act + var call = client.ServerStreamingCall(new DataMessage()); + + // Assert + var headers = await call.ResponseHeadersAsync; + var keyHeaders = headers.Where(k => k.Key == "key").ToList(); + Assert.AreEqual("key", keyHeaders[0].Key); + Assert.AreEqual("Value1", keyHeaders[0].Value); + Assert.AreEqual("key", keyHeaders[1].Key); + Assert.AreEqual("Value2", keyHeaders[1].Value); + + Assert.IsTrue(await call.ResponseStream.MoveNext().DefaultTimeout()); + Assert.IsTrue(await call.ResponseStream.MoveNext().DefaultTimeout()); + Assert.IsTrue(await call.ResponseStream.MoveNext().DefaultTimeout()); + Assert.IsTrue(await call.ResponseStream.MoveNext().DefaultTimeout()); + + var ex = await ExceptionAssert.ThrowsAsync(() => call.ResponseStream.MoveNext()).DefaultTimeout(); + + var trailers = call.GetTrailers(); + Assert.AreEqual(2, trailers.Count); + Assert.AreEqual("key", trailers[0].Key); + Assert.AreEqual("ResponseTrailers", trailers[0].Value); + Assert.AreEqual("key", trailers[1].Key); + Assert.AreEqual("RpcException", trailers[1].Value); + + Assert.AreEqual(StatusCode.Aborted, call.GetStatus().StatusCode); + Assert.AreEqual("Message", call.GetStatus().Detail); + } + private static byte[] CreateTestData(int size) { var data = new byte[size]; From 490ff1a8631c6373e6db8dd1c039f8857b1a3022 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 21 May 2020 23:34:31 +1200 Subject: [PATCH 2/2] Clean up --- .../Internal/HttpContextServerCallContext.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs b/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs index 636e249a8..479a4a983 100644 --- a/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs +++ b/src/Grpc.AspNetCore.Server/Internal/HttpContextServerCallContext.cs @@ -362,14 +362,8 @@ protected override Task WriteResponseHeadersAsyncCore(Metadata responseHeaders) } else { - if (entry.IsBinary) - { - HttpContext.Response.Headers.Append(entry.Key, Convert.ToBase64String(entry.ValueBytes)); - } - else - { - HttpContext.Response.Headers.Append(entry.Key, entry.Value); - } + var encodedValue = entry.IsBinary ? Convert.ToBase64String(entry.ValueBytes) : entry.Value; + HttpContext.Response.Headers.Append(entry.Key, encodedValue); } } }