Skip to content

Commit 0ac8d00

Browse files
committed
Add Content to all HttpResponseMessages
- also, do _not_ require `Stream.CanSeek` when calculating `HttpMessageContent.ContentLength` value - 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
1 parent 537a0f8 commit 0ac8d00

File tree

10 files changed

+29
-39
lines changed

10 files changed

+29
-39
lines changed

src/System.Net.Http.Formatting/HttpContentMessageExtensions.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ private static HttpContent CreateHeaderFields(HttpHeaders source, HttpHeaders de
451451
Contract.Assert(destination != null, "destination headers cannot be null");
452452
Contract.Assert(contentStream != null, "contentStream must be non null");
453453
HttpContentHeaders contentHeaders = null;
454-
HttpContent content = null;
454+
HttpContent content;
455455

456456
// Set the header fields
457457
foreach (KeyValuePair<string, IEnumerable<string>> header in source)
@@ -481,6 +481,10 @@ private static HttpContent CreateHeaderFields(HttpHeaders source, HttpHeaders de
481481
content = new StreamContent(contentStream);
482482
contentHeaders.CopyTo(content.Headers);
483483
}
484+
else
485+
{
486+
content = new StreamContent(Stream.Null);
487+
}
484488

485489
return content;
486490
}

src/System.Net.Http.Formatting/HttpMessageContent.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ protected override async Task SerializeToStreamAsync(Stream stream, TransportCon
190190
if (Content != null)
191191
{
192192
Stream readStream = await _streamTask.Value;
193-
ValidateStreamForReading(readStream);
193+
ValidateStreamForReading(readStream); // ???
194194
await Content.CopyToAsync(stream);
195195
}
196196
}
@@ -205,6 +205,7 @@ protected override async Task SerializeToStreamAsync(Stream stream, TransportCon
205205
protected override bool TryComputeLength(out long length)
206206
{
207207
// We have four states we could be in:
208+
// 0. We have content and it knows its ContentLength.
208209
// 1. We have content, but the task is still running or finished without success
209210
// 2. We have content, the task has finished successfully, and the stream came back as a null or non-seekable
210211
// 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)
214215
// For #3, we return true & the size of our headers + the content length
215216
// For #4, we return true & the size of our headers
216217

217-
bool hasContent = _streamTask.Value != null;
218218
length = 0;
219219

220-
// Cases #1, #2, #3
221-
if (hasContent)
220+
if (Content?.Headers.ContentLength is not null)
221+
{
222+
length = (long)Content.Headers.ContentLength; // Case #0
223+
}
224+
else if (_streamTask.Value is not null)
222225
{
223226
Stream readStream;
224227
if (!_streamTask.Value.TryGetResult(out readStream) // Case #1
@@ -354,7 +357,7 @@ private byte[] SerializeHeader()
354357
return Encoding.UTF8.GetBytes(message.ToString());
355358
}
356359

357-
private void ValidateStreamForReading(Stream stream)
360+
private void ValidateStreamForReading(Stream stream) // ???
358361
{
359362
// If the content needs to be written to a target stream a 2nd time, then the stream must support
360363
// seeking (e.g. a FileStream), otherwise the stream can't be copied a second time to a target

src/System.Net.Http.Formatting/HttpRequestMessageExtensions.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.ComponentModel;
55
using System.Diagnostics.CodeAnalysis;
6+
using System.IO;
67
using System.Web.Http;
78

89
namespace System.Net.Http
@@ -29,6 +30,7 @@ public static HttpResponseMessage CreateResponse(this HttpRequestMessage request
2930

3031
return new HttpResponseMessage
3132
{
33+
Content = new StreamContent(Stream.Null),
3234
StatusCode = statusCode,
3335
RequestMessage = request
3436
};
@@ -49,6 +51,7 @@ public static HttpResponseMessage CreateResponse(this HttpRequestMessage request
4951

5052
return new HttpResponseMessage
5153
{
54+
Content = new StreamContent(Stream.Null),
5255
RequestMessage = request
5356
};
5457
}

test/System.Net.Http.Formatting.Test/Formatting/JsonMediaTypeFormatterTests.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,10 @@
1111
using System.Text;
1212
using System.Threading.Tasks;
1313
using Microsoft.TestCommon;
14-
using Moq;
1514
using Newtonsoft.Json;
1615
using Newtonsoft.Json.Linq;
1716

18-
namespace System.Net.Http.Formatting
19-
{
17+
namespace System.Net.Http.Formatting {
2018
public class JsonMediaTypeFormatterTests : MediaTypeFormatterTestBase<JsonMediaTypeFormatter>
2119
{
2220
// Test data which should round-trip without type information in the serialization. Contains an exhaustive
@@ -588,7 +586,7 @@ public override async Task WriteToStreamAsync_WhenObjectIsNull_WritesDataButDoes
588586
// Arrange
589587
JsonMediaTypeFormatter formatter = CreateFormatter();
590588
Stream stream = new MemoryStream();
591-
HttpContent content = new StringContent(String.Empty);
589+
HttpContent content = new StreamContent(Stream.Null);
592590

593591
// Act
594592
await formatter.WriteToStreamAsync(typeof(SampleType), null, stream, content, null);

test/System.Net.Http.Formatting.Test/Handlers/ProgressMessageHandlerTest.cs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,8 @@ public async Task SendAsync_InsertsReceiveProgressWhenResponseEntityPresent(bool
8484
}
8585
else
8686
{
87-
#if NET6_0_OR_GREATER
88-
// Test initialization (request.CreateResponse() near the end of this file) for this case leaves
89-
// response.Content non-null (an EmptyContent or StreamContent instance). Those types are internal.
90-
// Just confirm the length matches expectations.
9187
Assert.NotNull(response.Content);
9288
Assert.Equal(0L, response.Content.Headers.ContentLength);
93-
#else
94-
Assert.Null(response.Content);
95-
#endif
9689
}
9790
}
9891
}

test/System.Net.Http.Formatting.Test/Handlers/ProgressStreamTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ internal static ProgressStream CreateProgressStream(
274274
Stream iStream = innerStream ?? new Mock<Stream>().Object;
275275
ProgressMessageHandler pHandler = progressMessageHandler ?? new ProgressMessageHandler();
276276
HttpRequestMessage req = request ?? new HttpRequestMessage();
277-
HttpResponseMessage rsp = response ?? new HttpResponseMessage();
277+
HttpResponseMessage rsp = response ?? new HttpResponseMessage() { Content = new StreamContent(Stream.Null) };
278278
return new ProgressStream(iStream, pHandler, req, rsp);
279279
}
280280

test/System.Net.Http.Formatting.Test/HttpMessageContentTests.cs

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4+
using System.IO;
45
using System.Net.Http.Headers;
56
using System.Threading.Tasks;
67
using Microsoft.TestCommon;
@@ -39,10 +40,8 @@ private static HttpResponseMessage CreateResponse(bool containsEntity)
3940
httpResponse.ReasonPhrase = ParserData.HttpReasonPhrase;
4041
httpResponse.Version = new Version("1.2");
4142
AddMessageHeaders(httpResponse.Headers);
42-
if (containsEntity)
43-
{
44-
httpResponse.Content = new StringContent(ParserData.HttpMessageEntity);
45-
}
43+
httpResponse.Content =
44+
containsEntity ? new StringContent(ParserData.HttpMessageEntity) : new StreamContent(Stream.Null);
4645

4746
return httpResponse;
4847
}
@@ -77,20 +76,6 @@ private static async Task ValidateResponse(HttpContent content, bool containsEnt
7776
{
7877
Assert.Equal(ParserData.HttpResponseMediaType, content.Headers.ContentType);
7978

80-
#if NET6_0_OR_GREATER
81-
// Test initialization leaves the inner HttpContent (EmptyContent or StreamContent) w/ a non-null but
82-
// unseekable Stream (EmptyReadStream). HttpMessageContent believes the message has content but is unable
83-
// to calculate the message length correctly in this case. Overriding the Content e.g. w/ a StringContent
84-
// makes things worse because the serialized content then contains unexpected Content-Type and
85-
// Content-Length headers i.e. ParserData.HttpResponseWithEntity looks _smaller_ than
86-
// ParserData.HttpResponse though the opposite would be true in any real scenario. Buffer the
87-
// HttpMessageContent to give it a better chance at calculating the length.
88-
if (!containsEntity)
89-
{
90-
await content.LoadIntoBufferAsync();
91-
}
92-
#endif
93-
9479
long? length = content.Headers.ContentLength;
9580
Assert.NotNull(length);
9681

test/System.Net.Http.Formatting.Test/ParserData.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,15 @@ public static TheoryDataSet<string> InvalidStatusCodes
198198
((int)HttpStatus).ToString() +
199199
" " +
200200
HttpReasonPhrase +
201-
"\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\n\r\n";
201+
"\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\nContent-Length: 0\r\n\r\n";
202202

203203
public static readonly string HttpRequestWithEntity =
204204
HttpMethod +
205205
" /some/path HTTP/1.2\r\nHost: " +
206206
HttpHostName +
207207
"\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\nContent-Type: " +
208208
TextContentType +
209+
"\r\nContent-Length: 100" +
209210
"\r\n\r\n" +
210211
HttpMessageEntity;
211212

@@ -216,6 +217,7 @@ public static TheoryDataSet<string> InvalidStatusCodes
216217
HttpReasonPhrase +
217218
"\r\nN1: V1a, V1b, V1c, V1d, V1e\r\nN2: V2\r\nContent-Type: " +
218219
TextContentType +
220+
"\r\nContent-Length: 100" +
219221
"\r\n\r\n" +
220222
HttpMessageEntity;
221223
}

test/System.Web.Http.Test/Controllers/VoidResultConverterTest.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ public void Convert_ReturnsResponseMessageWithRequestAssignedAndNoContentToRefle
3131
var result = _converter.Convert(_context, null);
3232

3333
Assert.Equal(HttpStatusCode.NoContent, result.StatusCode);
34-
Assert.Null(result.Content);
34+
Assert.NotNull(result.Content);
35+
Assert.Equal(0L, result.Content.Headers.ContentLength);
3536
Assert.Same(_request, result.RequestMessage);
3637
}
3738
}

test/System.Web.Http.WebHost.Test/WebHostExceptionHandlerTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,8 @@ public async Task HandleAsync_IfCatchBlockIsWebHostBufferedContent_WithCreateExc
220220
{
221221
Assert.NotNull(response);
222222
Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
223-
Assert.Null(response.Content);
223+
Assert.NotNull(response.Content);
224+
Assert.Equal(0L, response.Content.Headers.ContentLength);
224225
Assert.Same(expectedRequest, response.RequestMessage);
225226
}
226227
}

0 commit comments

Comments
 (0)