Make HttpResponseMessage.Content non-nullable (#35910)
authorStephen Toub <stoub@microsoft.com>
Fri, 8 May 2020 00:02:37 +0000 (20:02 -0400)
committerGitHub <noreply@github.com>
Fri, 8 May 2020 00:02:37 +0000 (20:02 -0400)
* Make HttpResponseMessage.Content non-nullable

* Fix Microsoft.Extensions.Http test

src/libraries/Microsoft.Extensions.Http/tests/Logging/RedactedLogValueIntegrationTest.cs
src/libraries/System.Net.Http/ref/System.Net.Http.cs
src/libraries/System.Net.Http/src/System.Net.Http.csproj
src/libraries/System.Net.Http/src/System/Net/Http/EmptyContent.cs [new file with mode: 0644]
src/libraries/System.Net.Http/src/System/Net/Http/EmptyReadStream.cs [moved from src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/EmptyReadStream.cs with 100% similarity]
src/libraries/System.Net.Http/src/System/Net/Http/HttpBaseStream.cs [moved from src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpBaseStream.cs with 100% similarity]
src/libraries/System.Net.Http/src/System/Net/Http/HttpResponseMessage.cs
src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientTest.cs
src/libraries/System.Net.Http/tests/FunctionalTests/HttpResponseMessageTest.cs
src/libraries/System.Net.Http/tests/StressTests/HttpStress/ClientOperations.cs
src/libraries/System.Net.Http/tests/UnitTests/System.Net.Http.Unit.Tests.csproj

index 67ec296..3b3c122 100644 (file)
@@ -51,7 +51,7 @@ namespace Microsoft.Extensions.Http.Logging
                     m.EventId == LoggingScopeHttpMessageHandler.Log.EventIds.RequestHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Request Headers:
 Authorization: *
 Cache-Control: no-cache
@@ -63,7 +63,7 @@ Cache-Control: no-cache
                     m.EventId == LoggingHttpMessageHandler.Log.EventIds.RequestHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Request Headers:
 Authorization: *
 Cache-Control: no-cache
@@ -75,7 +75,7 @@ Cache-Control: no-cache
                     m.EventId == LoggingHttpMessageHandler.Log.EventIds.ResponseHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Response Headers:
 X-Sensitive: *
 Y-Non-Sensitive: innocuous value
@@ -87,7 +87,7 @@ Y-Non-Sensitive: innocuous value
                     m.EventId == LoggingScopeHttpMessageHandler.Log.EventIds.ResponseHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Response Headers:
 X-Sensitive: *
 Y-Non-Sensitive: innocuous value
@@ -132,7 +132,7 @@ Y-Non-Sensitive: innocuous value
                     m.EventId == LoggingScopeHttpMessageHandler.Log.EventIds.RequestHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Request Headers:
 Authorization: *
 Cache-Control: no-cache
@@ -144,7 +144,7 @@ Cache-Control: no-cache
                     m.EventId == LoggingHttpMessageHandler.Log.EventIds.RequestHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Request Headers:
 Authorization: *
 Cache-Control: no-cache
@@ -156,7 +156,7 @@ Cache-Control: no-cache
                     m.EventId == LoggingHttpMessageHandler.Log.EventIds.ResponseHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Response Headers:
 X-Sensitive: *
 Y-Non-Sensitive: innocuous value
@@ -168,7 +168,7 @@ Y-Non-Sensitive: innocuous value
                     m.EventId == LoggingScopeHttpMessageHandler.Log.EventIds.ResponseHeader &&
                     m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
             }));
-            Assert.Equal(
+            Assert.StartsWith(
 @"Response Headers:
 X-Sensitive: *
 Y-Non-Sensitive: innocuous value
index 0e07521..3026fa7 100644 (file)
@@ -209,7 +209,8 @@ namespace System.Net.Http
     {
         public HttpResponseMessage() { }
         public HttpResponseMessage(System.Net.HttpStatusCode statusCode) { }
-        public System.Net.Http.HttpContent? Content { get { throw null; } set { } }
+        [System.Diagnostics.CodeAnalysis.AllowNull]
+        public System.Net.Http.HttpContent Content { get { throw null; } set { } }
         public System.Net.Http.Headers.HttpResponseHeaders Headers { get { throw null; } }
         public bool IsSuccessStatusCode { get { throw null; } }
         public string? ReasonPhrase { get { throw null; } set { } }
index 0744c3d..18b5292 100644 (file)
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <OutputType>Library</OutputType>
     <AssemblyName>System.Net.Http</AssemblyName>
     <Compile Include="System\Net\Http\ByteArrayHelpers.cs" />
     <Compile Include="System\Net\Http\ClientCertificateOption.cs" />
     <Compile Include="System\Net\Http\DelegatingHandler.cs" />
+    <Compile Include="System\Net\Http\EmptyContent.cs" />
+    <Compile Include="System\Net\Http\EmptyReadStream.cs" />
     <Compile Include="System\Net\Http\FormUrlEncodedContent.cs" />
     <Compile Include="System\Net\Http\Headers\AltSvcHeaderParser.cs" />
     <Compile Include="System\Net\Http\Headers\AltSvcHeaderValue.cs" />
     <Compile Include="System\Net\Http\Headers\KnownHeader.cs" />
     <Compile Include="System\Net\Http\Headers\HttpHeaderType.cs" />
     <Compile Include="System\Net\Http\Headers\KnownHeaders.cs" />
+    <Compile Include="System\Net\Http\HttpBaseStream.cs" />
     <Compile Include="System\Net\Http\HttpClient.cs" />
     <Compile Include="System\Net\Http\HttpClientHandler.cs" />
     <Compile Include="System\Net\Http\HttpClientHandler.Core.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\CreditManager.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\CreditWaiter.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\DecompressionHandler.cs" />
-    <Compile Include="System\Net\Http\SocketsHttpHandler\EmptyReadStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\FailedProxyCache.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\Http2Connection.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\Http2ConnectionException.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\Http3RequestStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpAuthenticatedConnectionHandler.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpAuthority.cs" />
-    <Compile Include="System\Net\Http\SocketsHttpHandler\HttpBaseStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnection.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionBase.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpConnectionHandler.cs" />
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/EmptyContent.cs b/src/libraries/System.Net.Http/src/System/Net/Http/EmptyContent.cs
new file mode 100644 (file)
index 0000000..7282293
--- /dev/null
@@ -0,0 +1,38 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.IO;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace System.Net.Http
+{
+    /// <summary>Provides a zero-length HttpContent implementation.</summary>
+    internal sealed class EmptyContent : HttpContent
+    {
+        protected internal override bool TryComputeLength(out long length)
+        {
+            length = 0;
+            return true;
+        }
+
+        protected override Task SerializeToStreamAsync(Stream stream, TransportContext? context) =>
+            Task.CompletedTask;
+
+        protected override Task SerializeToStreamAsync(Stream stream, TransportContext? context, CancellationToken cancellationToken) =>
+            cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) :
+            SerializeToStreamAsync(stream, context);
+
+        protected override Task<Stream> CreateContentReadStreamAsync() =>
+            Task.FromResult<Stream>(EmptyReadStream.Instance);
+
+        protected override Task<Stream> CreateContentReadStreamAsync(CancellationToken cancellationToken) =>
+            cancellationToken.IsCancellationRequested ? Task.FromCanceled<Stream>(cancellationToken) :
+            CreateContentReadStreamAsync();
+
+        internal override Stream? TryCreateContentReadStream() => EmptyReadStream.Instance;
+
+        internal override bool AllowDuplex => false;
+    }
+}
index dbe85b5..b0763db 100644 (file)
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Diagnostics.CodeAnalysis;
 using System.Net.Http.Headers;
 using System.Text;
 
@@ -39,9 +40,10 @@ namespace System.Net.Http
 
         internal void SetVersionWithoutValidation(Version value) => _version = value;
 
-        public HttpContent? Content
+        [AllowNull]
+        public HttpContent Content
         {
-            get { return _content; }
+            get { return _content ??= new EmptyContent(); }
             set
             {
                 CheckDisposed();
index a297e28..544f188 100644 (file)
@@ -221,7 +221,10 @@ namespace System.Net.Http.Functional.Tests
             {
                 Assert.Same(string.Empty, await client.GetStringAsync(CreateFakeUri()));
                 Assert.Same(Array.Empty<byte>(), await client.GetByteArrayAsync(CreateFakeUri()));
-                Assert.Same(Stream.Null, await client.GetStreamAsync(CreateFakeUri()));
+
+                Stream s = await client.GetStreamAsync(CreateFakeUri());
+                Assert.NotNull(s);
+                Assert.Equal(-1, s.ReadByte());
             }
         }
 
index fdeeeb0..7c7fafe 100644 (file)
@@ -2,10 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System;
 using System.IO;
-using System.Net.Http.Headers;
-using System.Threading;
 using System.Threading.Tasks;
 
 using Xunit;
@@ -22,7 +19,7 @@ namespace System.Net.Http.Functional.Tests
                 Assert.Equal(HttpStatusCode.OK, rm.StatusCode);
                 Assert.Equal("OK", rm.ReasonPhrase);
                 Assert.Equal(new Version(1, 1), rm.Version);
-                Assert.Null(rm.Content);
+                Assert.NotNull(rm.Content);
                 Assert.Null(rm.RequestMessage);
             }
         }
@@ -35,7 +32,7 @@ namespace System.Net.Http.Functional.Tests
                 Assert.Equal(HttpStatusCode.Accepted, rm.StatusCode);
                 Assert.Equal("Accepted", rm.ReasonPhrase);
                 Assert.Equal(new Version(1, 1), rm.Version);
-                Assert.Null(rm.Content);
+                Assert.NotNull(rm.Content);
                 Assert.Null(rm.RequestMessage);
             }
         }
@@ -232,8 +229,15 @@ namespace System.Net.Http.Functional.Tests
         {
             using (var rm = new HttpResponseMessage())
             {
+                HttpContent c1 = rm.Content;
+                Assert.Same(c1, rm.Content);
+
                 rm.Content = null;
-                Assert.Null(rm.Content);
+
+                HttpContent c2 = rm.Content;
+                Assert.Same(c2, rm.Content);
+
+                Assert.NotSame(c1, c2);
             }
         }
 
@@ -250,6 +254,35 @@ namespace System.Net.Http.Functional.Tests
         }
 
         [Fact]
+        public async Task DefaultContent_ReadableNotWritable_Success()
+        {
+            var resp = new HttpResponseMessage();
+
+            HttpContent c = resp.Content;
+            Assert.NotNull(c);
+            Assert.Same(c, resp.Content);
+            Assert.NotSame(resp.Content, new HttpResponseMessage().Content);
+
+            Assert.Equal(0, c.Headers.ContentLength);
+
+            Task<Stream> t = c.ReadAsStreamAsync();
+            Assert.Equal(TaskStatus.RanToCompletion, t.Status);
+
+            Stream s = await t;
+            Assert.NotNull(s);
+
+            Assert.Equal(-1, s.ReadByte());
+            Assert.Equal(0, s.Read(new byte[1], 0, 1));
+            Assert.Equal(0, await s.ReadAsync(new byte[1], 0, 1));
+            Assert.Equal(0, await s.ReadAsync(new Memory<byte>(new byte[1])));
+
+            Assert.Throws<NotSupportedException>(() => s.WriteByte(0));
+            Assert.Throws<NotSupportedException>(() => s.Write(new byte[1], 0, 1));
+            await Assert.ThrowsAsync<NotSupportedException>(() => s.WriteAsync(new byte[1], 0, 1));
+            await Assert.ThrowsAsync<NotSupportedException>(async () => await s.WriteAsync(new ReadOnlyMemory<byte>(new byte[1])));
+        }
+
+        [Fact]
         public void ToString_DefaultAndNonDefaultInstance_DumpAllFields()
         {
             using (var rm = new HttpResponseMessage())
index 2e148dd..8342fed 100644 (file)
@@ -192,7 +192,7 @@ namespace HttpStress
                     using HttpResponseMessage m = await ctx.SendAsync(req);
                     
                     ValidateStatusCode(m);
-                    ValidateServerContent(await m.Content!.ReadAsStringAsync(), expectedLength);
+                    ValidateServerContent(await m.Content.ReadAsStringAsync(), expectedLength);
                 }),
 
                 ("GET Partial",
@@ -204,7 +204,7 @@ namespace HttpStress
 
                     ValidateStatusCode(m);
 
-                    using (Stream s = await m.Content!.ReadAsStreamAsync())
+                    using (Stream s = await m.Content.ReadAsStreamAsync())
                     {
                         s.ReadByte(); // read single byte from response and throw the rest away
                     }
@@ -221,7 +221,7 @@ namespace HttpStress
 
                     ValidateStatusCode(res);
 
-                    await res.Content!.ReadAsStringAsync();
+                    await res.Content.ReadAsStringAsync();
 
                     bool isValidChecksum = ValidateServerChecksum(res.Headers, expectedChecksum);
                     string failureDetails = isValidChecksum ? "server checksum matches client checksum" : "server checksum mismatch";
@@ -273,7 +273,7 @@ namespace HttpStress
                     using HttpResponseMessage m = await ctx.SendAsync(req);
 
                     ValidateStatusCode(m);
-                    ValidateContent(expectedResponse, await m.Content!.ReadAsStringAsync(), $"Uri: {uri}");
+                    ValidateContent(expectedResponse, await m.Content.ReadAsStringAsync(), $"Uri: {uri}");
                 }),
 
                 ("GET Aborted",
@@ -332,7 +332,7 @@ namespace HttpStress
 
                     ValidateStatusCode(m);
                     string checksumMessage = ValidateServerChecksum(m.Headers, checksum) ? "server checksum matches client checksum" : "server checksum mismatch";
-                    ValidateContent(content, await m.Content!.ReadAsStringAsync(), checksumMessage);
+                    ValidateContent(content, await m.Content.ReadAsStringAsync(), checksumMessage);
                 }),
 
                 ("POST Multipart Data",
@@ -346,7 +346,7 @@ namespace HttpStress
 
                     ValidateStatusCode(m);
                     string checksumMessage = ValidateServerChecksum(m.Headers, checksum) ? "server checksum matches client checksum" : "server checksum mismatch";
-                    ValidateContent(formData.expected, await m.Content!.ReadAsStringAsync(), checksumMessage);
+                    ValidateContent(formData.expected, await m.Content.ReadAsStringAsync(), checksumMessage);
                 }),
 
                 ("POST Duplex",
@@ -359,7 +359,7 @@ namespace HttpStress
                     using HttpResponseMessage m = await ctx.SendAsync(req, HttpCompletionOption.ResponseHeadersRead);
 
                     ValidateStatusCode(m);
-                    string response = await m.Content!.ReadAsStringAsync();
+                    string response = await m.Content.ReadAsStringAsync();
 
                     string checksumMessage = ValidateServerChecksum(m.TrailingHeaders, checksum, required: false) ? "server checksum matches client checksum" : "server checksum mismatch";
                     ValidateContent(content, await m.Content.ReadAsStringAsync(), checksumMessage);
@@ -376,7 +376,7 @@ namespace HttpStress
                     using HttpResponseMessage m = await ctx.SendAsync(req, HttpCompletionOption.ResponseHeadersRead);
 
                     ValidateStatusCode(m);
-                    string response = await m.Content!.ReadAsStringAsync();
+                    string response = await m.Content.ReadAsStringAsync();
 
                     // trailing headers not supported for all servers, so do not require checksums
                     bool isValidChecksum = ValidateServerChecksum(m.TrailingHeaders, checksum, required: false);
@@ -416,7 +416,7 @@ namespace HttpStress
 
                     ValidateStatusCode(m);
                     string checksumMessage = ValidateServerChecksum(m.Headers, checksum) ? "server checksum matches client checksum" : "server checksum mismatch";
-                    ValidateContent(content, await m.Content!.ReadAsStringAsync(), checksumMessage);
+                    ValidateContent(content, await m.Content.ReadAsStringAsync(), checksumMessage);
                 }),
 
                 ("HEAD",
@@ -428,7 +428,7 @@ namespace HttpStress
 
                     ValidateStatusCode(m);
 
-                    if (m.Content!.Headers.ContentLength != expectedLength)
+                    if (m.Content.Headers.ContentLength != expectedLength)
                     {
                         throw new Exception($"Expected {expectedLength}, got {m.Content.Headers.ContentLength}");
                     }
@@ -446,7 +446,7 @@ namespace HttpStress
 
                     ValidateStatusCode(m);
 
-                    string r = await m.Content!.ReadAsStringAsync();
+                    string r = await m.Content.ReadAsStringAsync();
                     if (r != "") throw new Exception($"Got unexpected response: {r}");
                 }),
 
@@ -460,7 +460,7 @@ namespace HttpStress
 
                     ValidateStatusCode(m);
 
-                    string r = await m.Content!.ReadAsStringAsync();
+                    string r = await m.Content.ReadAsStringAsync();
                     if (r != "") throw new Exception($"Got unexpected response: {r}");
                 }),
 
@@ -472,7 +472,7 @@ namespace HttpStress
                     using HttpResponseMessage m = await ctx.SendAsync(req);
 
                     ValidateStatusCode(m);
-                    ValidateServerContent(await m.Content!.ReadAsStringAsync(), expectedLength);
+                    ValidateServerContent(await m.Content.ReadAsStringAsync(), expectedLength);
                 }),
             };
 
index c14ef4c..b4c31ba 100644 (file)
@@ -70,6 +70,8 @@
              Link="ProductionCode\Common\System\Threading\Tasks\TaskToApm.cs" />
     <Compile Include="..\..\src\System\Net\Http\SocketsHttpHandler\AuthenticationHelper.Digest.cs"
              Link="ProductionCode\System\Net\Http\SocketsHttpHandler\AuthenticationHelper.Digest.cs" />
+    <Compile Include="..\..\src\System\Net\Http\HttpBaseStream.cs"
+             Link="ProductionCode\System\Net\Http\HttpBaseStream.cs" />
     <Compile Include="..\..\src\System\Net\Http\ByteArrayContent.cs"
              Link="ProductionCode\System\Net\Http\ByteArrayContent.cs" />
     <Compile Include="..\..\src\System\Net\Http\ClientCertificateOption.cs"
              Link="ProductionCode\System\IO\DelegatingStream.cs" />
     <Compile Include="..\..\src\System\Net\Http\ByteArrayHelpers.cs"
              Link="ProductionCode\System\Net\Http\ByteArrayHelpers.cs" />
+    <Compile Include="..\..\src\System\Net\Http\EmptyContent.cs"
+             Link="ProductionCode\System\Net\Http\EmptyContent.cs" />
+    <Compile Include="..\..\src\System\Net\Http\EmptyReadStream.cs"
+         Link="ProductionCode\System\Net\Http\EmptyReadStream.cs" />
     <Compile Include="..\..\src\System\Net\Http\FormUrlEncodedContent.cs"
              Link="ProductionCode\System\Net\Http\FormUrlEncodedContent.cs" />
     <Compile Include="..\..\src\System\Net\Http\Headers\AltSvcHeaderParser.cs"