Add Content to all HttpResponseMessages (#387)

- fix #386 - reenable `net6.0` tests disabled in #384 for this issue - also, check `Stream.CanSeek` less when calculating `HttpMessageContent.ContentLength` value - use inner `HttpContent.Headers.ContentLength` if available - this changes behaviour slightly for both requests and responses - observable as `HttpMessageContent.Headers.ContentLength!=null` for empty messages - for responses, avoids `ContentLength==null` versus `ContentLength==0` inconsistencies (depending on platform) - `ContentLength==null` may still occur in some corner cases (which existed before) - e.g. when inner `HttpContent` doesn't know its length and `ReadAsStreamAsync()` hasn't completed - main change means `HttpResponseMessage`s we expose to user code are consistent across platforms - note: user code won't see `EmptyContent` in `HttpResponseMessage`s we create
aspnet · Feb 17, 2023 · 979e6ac · 979e6ac
1 parent 0f4c624
commit 979e6ac
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 20 deletions.
diff --git a/src/System.Net.Http.Formatting/HttpContentMessageExtensions.cs b/src/System.Net.Http.Formatting/HttpContentMessageExtensions.cs
@@ -451,7 +451,7 @@ private static HttpContent CreateHeaderFields(HttpHeaders source, HttpHeaders de
             Contract.Assert(destination != null, "destination headers cannot be null");
             Contract.Assert(contentStream != null, "contentStream must be non null");
             HttpContentHeaders contentHeaders = null;
-            HttpContent content = null;
+            HttpContent content;
 
             // Set the header fields
             foreach (KeyValuePair<string, IEnumerable<string>> header in source)
@@ -481,6 +481,10 @@ private static HttpContent CreateHeaderFields(HttpHeaders source, HttpHeaders de
                 content = new StreamContent(contentStream);
                 contentHeaders.CopyTo(content.Headers);
             }
+            else
+            {
+                content = new StreamContent(Stream.Null);
+            }
 
             return content;
         }

diff --git a/src/System.Net.Http.Formatting/HttpMessageContent.cs b/src/System.Net.Http.Formatting/HttpMessageContent.cs
@@ -205,6 +205,7 @@ protected override async Task SerializeToStreamAsync(Stream stream, TransportCon
         protected override bool TryComputeLength(out long length)
         {
             // We have four states we could be in:
+            //   0. We have content and it knows its ContentLength.
             //   1. We have content, but the task is still running or finished without success
             //   2. We have content, the task has finished successfully, and the stream came back as a null or non-seekable
             //   3. We have content, the task has finished successfully, and the stream is seekable, so we know its length
@@ -214,11 +215,13 @@ protected override bool TryComputeLength(out long length)
             // For #3, we return true & the size of our headers + the content length
             // For #4, we return true & the size of our headers
 
-            bool hasContent = _streamTask.Value != null;
             length = 0;
 
-            // Cases #1, #2, #3
-            if (hasContent)
+            if (Content?.Headers.ContentLength is not null)
+            {
+                length = (long)Content.Headers.ContentLength; // Case #0
+            }
+            else if (_streamTask.Value is not null)
             {
                 Stream readStream;
                 if (!_streamTask.Value.TryGetResult(out readStream) // Case #1

diff --git a/src/System.Net.Http.Formatting/HttpRequestMessageExtensions.cs b/src/System.Net.Http.Formatting/HttpRequestMessageExtensions.cs
@@ -3,6 +3,7 @@
 
 using System.ComponentModel;
 using System.Diagnostics.CodeAnalysis;
+using System.IO;
 using System.Web.Http;
 
 namespace System.Net.Http
@@ -29,6 +30,7 @@ public static HttpResponseMessage CreateResponse(this HttpRequestMessage request
 
             return new HttpResponseMessage
             {
+                Content = new StreamContent(Stream.Null),
                 StatusCode = statusCode,
                 RequestMessage = request
             };
@@ -49,6 +51,7 @@ public static HttpResponseMessage CreateResponse(this HttpRequestMessage request
 
             return new HttpResponseMessage
             {
+                Content = new StreamContent(Stream.Null),
                 RequestMessage = request
             };
         }

diff --git a/test/System.Net.Http.Formatting.Test/Formatting/JsonMediaTypeFormatterTests.cs b/test/System.Net.Http.Formatting.Test/Formatting/JsonMediaTypeFormatterTests.cs
@@ -588,7 +588,7 @@ public override async Task WriteToStreamAsync_WhenObjectIsNull_WritesDataButDoes
             // Arrange
             JsonMediaTypeFormatter formatter = CreateFormatter();
             Stream stream = new MemoryStream();
-            HttpContent content = new StringContent(String.Empty);
+            HttpContent content = new StreamContent(Stream.Null);
 
             // Act
             await formatter.WriteToStreamAsync(typeof(SampleType), null, stream, content, null);

diff --git a/test/System.Net.Http.Formatting.Test/Handlers/ProgressMessageHandlerTest.cs b/test/System.Net.Http.Formatting.Test/Handlers/ProgressMessageHandlerTest.cs
@@ -56,10 +56,8 @@ public async Task SendAsync_DoesNotInsertSendProgressWithoutEntityOrHandlerPrese
         }
 
         [Theory]
-#if !NET6_0_OR_GREATER // https://github.com/aspnet/AspNetWebStack/issues/386
         [InlineData(false, false)]
         [InlineData(false, true)]
-#endif
         [InlineData(true, false)]
         [InlineData(true, true)]
         public async Task SendAsync_InsertsReceiveProgressWhenResponseEntityPresent(bool insertResponseEntity, bool addReceiveProgressHandler)
@@ -86,7 +84,8 @@ public async Task SendAsync_InsertsReceiveProgressWhenResponseEntityPresent(bool
                 }
                 else
                 {
-                    Assert.Null(response.Content);
+                    Assert.NotNull(response.Content);
+                    Assert.Equal(0L, response.Content.Headers.ContentLength);
                 }
             }
         }

diff --git a/test/System.Net.Http.Formatting.Test/Handlers/ProgressStreamTest.cs b/test/System.Net.Http.Formatting.Test/Handlers/ProgressStreamTest.cs
@@ -274,7 +274,7 @@ internal static ProgressStream CreateProgressStream(
             Stream iStream = innerStream ?? new Mock<Stream>().Object;
             ProgressMessageHandler pHandler = progressMessageHandler ?? new ProgressMessageHandler();
             HttpRequestMessage req = request ?? new HttpRequestMessage();
-            HttpResponseMessage rsp = response ?? new HttpResponseMessage();
+            HttpResponseMessage rsp = response ?? new HttpResponseMessage() { Content = new StreamContent(Stream.Null) };
             return new ProgressStream(iStream, pHandler, req, rsp);
         }
 

diff --git a/test/System.Net.Http.Formatting.Test/HttpMessageContentTests.cs b/test/System.Net.Http.Formatting.Test/HttpMessageContentTests.cs
@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.IO;
 using System.Net.Http.Headers;
 using System.Threading.Tasks;
 using Microsoft.TestCommon;
@@ -39,10 +40,8 @@ private static HttpResponseMessage CreateResponse(bool containsEntity)
             httpResponse.ReasonPhrase = ParserData.HttpReasonPhrase;
             httpResponse.Version = new Version("1.2");
             AddMessageHeaders(httpResponse.Headers);
-            if (containsEntity)
-            {
-                httpResponse.Content = new StringContent(ParserData.HttpMessageEntity);
-            }
+            httpResponse.Content =
+                containsEntity ? new StringContent(ParserData.HttpMessageEntity) : new StreamContent(Stream.Null);
 
             return httpResponse;
         }
@@ -76,6 +75,7 @@ private static async Task ValidateRequest(HttpContent content, bool containsEnti
         private static async Task ValidateResponse(HttpContent content, bool containsEntity)
         {
             Assert.Equal(ParserData.HttpResponseMediaType, content.Headers.ContentType);
+
             long? length = content.Headers.ContentLength;
             Assert.NotNull(length);
 
@@ -164,7 +164,6 @@ public async Task SerializeRequestMultipleTimes()
             }
         }
 
-#if !NET6_0_OR_GREATER // https://github.com/aspnet/AspNetWebStack/issues/386
         [Fact]
         public async Task SerializeResponse()
         {
@@ -186,7 +185,6 @@ public async Task SerializeResponseMultipleTimes()
                 await ValidateResponse(instance, false);
             }
         }
-#endif
 
         [Fact]
         public async Task SerializeRequestWithEntity()
@@ -243,7 +241,6 @@ public async Task SerializeRequestAsync()
             }
         }
 
-#if !NET6_0_OR_GREATER // https://github.com/aspnet/AspNetWebStack/issues/386
         [Fact]
         public async Task SerializeResponseAsync()
         {
@@ -254,7 +251,6 @@ public async Task SerializeResponseAsync()
                 await ValidateResponse(instance, false);
             }
         }
-#endif
 
         [Fact]
         public async Task SerializeRequestWithPortAndQueryAsync()

diff --git a/test/System.Net.Http.Formatting.Test/ParserData.cs b/test/System.Net.Http.Formatting.Test/ParserData.cs
@@ -198,14 +198,15 @@ public static TheoryDataSet<string> InvalidStatusCodes
             ((int)HttpStatus).ToString() +
             " " +
             HttpReasonPhrase +
-            "\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\n\r\n";
+            "\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\nContent-Length: 0\r\n\r\n";
 
         public static readonly string HttpRequestWithEntity =
             HttpMethod +
             " /some/path HTTP/1.2\r\nHost: " +
             HttpHostName +
             "\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\nContent-Type: " +
             TextContentType +
+            "\r\nContent-Length: 100" +
             "\r\n\r\n" +
             HttpMessageEntity;
 
@@ -216,6 +217,7 @@ public static TheoryDataSet<string> InvalidStatusCodes
             HttpReasonPhrase +
             "\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\nContent-Type: " +
             TextContentType +
+            "\r\nContent-Length: 100" +
             "\r\n\r\n" +
             HttpMessageEntity;
     }

diff --git a/test/System.Web.Http.Test/Controllers/VoidResultConverterTest.cs b/test/System.Web.Http.Test/Controllers/VoidResultConverterTest.cs
@@ -31,7 +31,8 @@ public void Convert_ReturnsResponseMessageWithRequestAssignedAndNoContentToRefle
             var result = _converter.Convert(_context, null);
 
             Assert.Equal(HttpStatusCode.NoContent, result.StatusCode);
-            Assert.Null(result.Content);
+            Assert.NotNull(result.Content);
+            Assert.Equal(0L, result.Content.Headers.ContentLength);
             Assert.Same(_request, result.RequestMessage);
         }
     }

diff --git a/test/System.Web.Http.WebHost.Test/WebHostExceptionHandlerTests.cs b/test/System.Web.Http.WebHost.Test/WebHostExceptionHandlerTests.cs
@@ -220,7 +220,8 @@ public async Task HandleAsync_IfCatchBlockIsWebHostBufferedContent_WithCreateExc
                 {
                     Assert.NotNull(response);
                     Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
-                    Assert.Null(response.Content);
+                    Assert.NotNull(response.Content);
+                    Assert.Equal(0L, response.Content.Headers.ContentLength);
                     Assert.Same(expectedRequest, response.RequestMessage);
                 }
             }