From 391816eb7141569ad8bb70b23e3fd2fa327dbdcc Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Tue, 30 Dec 2014 16:26:08 -0800 Subject: [PATCH] [Fixes #1713]WebApiCompatShim doesn't work with Transfer Encoding: chunked --- .../HttpResponseMessageOutputFormatter.cs | 15 +++ .../WebApiCompatShimBasicTest.cs | 36 ++++-- ...HttpResponseMessageOutputFormatterTests.cs | 109 +++++++++++++++++- 3 files changed, 147 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Formatters/HttpResponseMessageOutputFormatter.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Formatters/HttpResponseMessageOutputFormatter.cs index 655e926ad5..3c12f904d2 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Formatters/HttpResponseMessageOutputFormatter.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/Formatters/HttpResponseMessageOutputFormatter.cs @@ -51,6 +51,15 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim } var responseHeaders = responseMessage.Headers; + + // Ignore the Transfer-Encoding header if it is just "chunked". + // We let the host decide about whether the response should be chunked or not. + if (responseHeaders.TransferEncodingChunked == true && + responseHeaders.TransferEncoding.Count == 1) + { + responseHeaders.TransferEncoding.Clear(); + } + foreach (var header in responseHeaders) { response.Headers.AppendValues(header.Key, header.Value.ToArray()); @@ -59,6 +68,12 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim if (responseMessage.Content != null) { var contentHeaders = responseMessage.Content.Headers; + + // Copy the response content headers only after ensuring they are complete. + // We ask for Content-Length first because HttpContent lazily computes this + // and only afterwards writes the value into the content headers. + var unused = contentHeaders.ContentLength; + foreach (var header in contentHeaders) { response.Headers.AppendValues(header.Key, header.Value.ToArray()); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/WebApiCompatShimBasicTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/WebApiCompatShimBasicTest.cs index 532fe2de37..181faded56 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/WebApiCompatShimBasicTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/WebApiCompatShimBasicTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.IO; using System.Net; using System.Net.Http; using System.Net.Http.Formatting; @@ -276,29 +277,44 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [Fact] - public async Task ApiController_ResponseReturned_Chunked() + public async Task ApiController_ExplicitChunkedEncoding_IsIgnored() { - // Arrange + // Arrange var server = TestServer.Create(_provider, _app); var client = server.CreateClient(); var expected = "POST Hello, HttpResponseMessage world!"; - // Act - var response = await client.PostAsync( - "http://localhost/api/Blog/HttpRequestMessage/EchoWithResponseMessageChunked", - new StringContent("Hello, HttpResponseMessage world!")); - var content = await response.Content.ReadAsStringAsync(); + // Act + var request = new HttpRequestMessage(); + request.Method = HttpMethod.Post; + request.RequestUri = new Uri("http://localhost/api/Blog/HttpRequestMessage/EchoWithResponseMessageChunked"); + request.Content = new StringContent("Hello, HttpResponseMessage world!"); - // Assert + // HttpClient buffers the response by default and this would set the Content-Length header and so + // this will not provide us accurate information as to whether the server set the header or + // the client. So here we explicitly mention to only read the headers and not the body. + var response = await client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); + + // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.Equal(expected, content); + Assert.NotNull(response.Content); + Assert.NotNull(response.Content.Headers.ContentLength); + Assert.Null(response.Headers.TransferEncodingChunked); + + // When HttpClient by default reads and buffers the resposne body, it diposes the + // response stream for us. But since we are reading the content explicitly, we need + // to close it. + var responseStream = await response.Content.ReadAsStreamAsync(); + using (var streamReader = new StreamReader(responseStream)) + { + Assert.Equal(expected, streamReader.ReadToEnd()); + } IEnumerable values; Assert.True(response.Headers.TryGetValues("X-Test", out values)); Assert.Equal(new string[] { "Hello!" }, values); - Assert.Equal(true, response.Headers.TransferEncodingChunked); } [Theory] diff --git a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpResponseMessageOutputFormatterTests.cs b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpResponseMessageOutputFormatterTests.cs index c90664d31c..6da3aace49 100644 --- a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpResponseMessageOutputFormatterTests.cs +++ b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpResponseMessageOutputFormatterTests.cs @@ -4,9 +4,12 @@ #if !ASPNETCORE50 using System; +using System.Linq; using System.IO; using System.Net.Http; +using System.Text; using System.Threading.Tasks; +using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.WebApiCompatShim; using Microsoft.AspNet.PipelineCore; using Moq; @@ -26,7 +29,10 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShimTest streamContent.Protected().Setup("Dispose", true).Verifiable(); var httpResponseMessage = new HttpResponseMessage(); httpResponseMessage.Content = streamContent.Object; - var outputFormatterContext = GetOutputFormatterContext(httpResponseMessage, typeof(HttpResponseMessage)); + var outputFormatterContext = GetOutputFormatterContext( + httpResponseMessage, + typeof(HttpResponseMessage), + new DefaultHttpContext()); // Act await formatter.WriteAsync(outputFormatterContext); @@ -35,13 +41,110 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShimTest streamContent.Protected().Verify("Dispose", Times.Once(), true); } - private OutputFormatterContext GetOutputFormatterContext(object outputValue, Type outputType) + [Fact] + public async Task ExplicitlySet_ChunkedEncodingFlag_IsIgnored() + { + // Arrange + var httpResponseMessage = new HttpResponseMessage(); + httpResponseMessage.Content = new StreamContent(new MemoryStream(Encoding.UTF8.GetBytes("Hello, World"))); + httpResponseMessage.Headers.TransferEncodingChunked = true; + + var httpContext = new DefaultHttpContext(); + var formatter = new HttpResponseMessageOutputFormatter(); + var outputFormatterContext = GetOutputFormatterContext( + httpResponseMessage, + typeof(HttpResponseMessage), + httpContext); + // Act + await formatter.WriteAsync(outputFormatterContext); + + // Assert + Assert.False(httpContext.Response.Headers.ContainsKey("Transfer-Encoding")); + Assert.NotNull(httpContext.Response.ContentLength); + } + + [Fact] + public async Task ExplicitlySet_ChunkedEncodingHeader_IsIgnored() + { + // Arrange + var transferEncodingHeaderKey = "Transfer-Encoding"; + var httpResponseMessage = new HttpResponseMessage(); + httpResponseMessage.Content = new StreamContent(new MemoryStream(Encoding.UTF8.GetBytes("Hello, World"))); + httpResponseMessage.Headers.Add(transferEncodingHeaderKey, "chunked"); + + var httpContext = new DefaultHttpContext(); + var formatter = new HttpResponseMessageOutputFormatter(); + var outputFormatterContext = GetOutputFormatterContext( + httpResponseMessage, + typeof(HttpResponseMessage), + httpContext); + // Act + await formatter.WriteAsync(outputFormatterContext); + + // Assert + Assert.False(httpContext.Response.Headers.ContainsKey(transferEncodingHeaderKey)); + Assert.NotNull(httpContext.Response.ContentLength); + } + + [Fact] + public async Task ExplicitlySet_MultipleEncodings_ChunkedNotIgnored() + { + // Arrange + var transferEncodingHeaderKey = "Transfer-Encoding"; + var httpResponseMessage = new HttpResponseMessage(); + httpResponseMessage.Content = new StreamContent(new MemoryStream(Encoding.UTF8.GetBytes("Hello, World"))); + httpResponseMessage.Headers.Add(transferEncodingHeaderKey, new[] { "identity", "chunked" }); + + var httpContext = new DefaultHttpContext(); + var formatter = new HttpResponseMessageOutputFormatter(); + var outputFormatterContext = GetOutputFormatterContext( + httpResponseMessage, + typeof(HttpResponseMessage), + httpContext); + // Act + await formatter.WriteAsync(outputFormatterContext); + + // Assert + Assert.True(httpContext.Response.Headers.ContainsKey(transferEncodingHeaderKey)); + Assert.Equal(new string[] { "identity", "chunked" }, + httpContext.Response.Headers.GetValues(transferEncodingHeaderKey)); + Assert.NotNull(httpContext.Response.ContentLength); + } + + [Fact] + public async Task ExplicitlySet_MultipleEncodingsUsingChunkedFlag_ChunkedNotIgnored() + { + // Arrange + var transferEncodingHeaderKey = "Transfer-Encoding"; + var httpResponseMessage = new HttpResponseMessage(); + httpResponseMessage.Content = new StreamContent(new MemoryStream(Encoding.UTF8.GetBytes("Hello, World"))); + httpResponseMessage.Headers.Add(transferEncodingHeaderKey, new[] { "identity" }); + httpResponseMessage.Headers.TransferEncodingChunked = true; + + var httpContext = new DefaultHttpContext(); + var formatter = new HttpResponseMessageOutputFormatter(); + var outputFormatterContext = GetOutputFormatterContext( + httpResponseMessage, + typeof(HttpResponseMessage), + httpContext); + // Act + await formatter.WriteAsync(outputFormatterContext); + + // Assert + Assert.True(httpContext.Response.Headers.ContainsKey(transferEncodingHeaderKey)); + Assert.Equal(new string[] { "identity", "chunked" }, + httpContext.Response.Headers.GetValues(transferEncodingHeaderKey)); + Assert.NotNull(httpContext.Response.ContentLength); + } + + private OutputFormatterContext GetOutputFormatterContext(object outputValue, Type outputType, + HttpContext httpContext) { return new OutputFormatterContext { Object = outputValue, DeclaredType = outputType, - ActionContext = new ActionContext(new DefaultHttpContext(), routeData: null, actionDescriptor: null) + ActionContext = new ActionContext(httpContext, routeData: null, actionDescriptor: null) }; } }