Managed Handler: add some basic protocol tests for status line handling

src/Common/tests/System/Net/Http/LoopbackServer.cs

@@ -29,6 +29,7 @@ public class Options
             public bool UseSsl { get; set; } = false;
             public SslProtocols SslProtocols { get; set; } = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12;
             public bool WebSocketEndpoint { get; set; } = false;
+            public Func<Stream, Stream> ResponseStreamWrapper { get; set; }
         }
 
         public static Task CreateServerAsync(Func<Socket, Uri, Task> funcAsync, Options options = null)
@@ -174,7 +175,7 @@ await sslStream.AuthenticateAsServerAsync(
                 }
 
                 using (var reader = new StreamReader(stream, Encoding.ASCII))
-                using (var writer = new StreamWriter(stream, Encoding.ASCII) { AutoFlush = true })
+                using (var writer = new StreamWriter(options?.ResponseStreamWrapper?.Invoke(stream) ?? stream, Encoding.ASCII) { AutoFlush = true })
                 {
                     return await funcAsync(s, stream, reader, writer).ConfigureAwait(false);
                 }

src/System.Net.Http/src/System/Net/Http/Managed/HttpConnection.cs

@@ -520,9 +520,16 @@ private void ParseStatusLine(Span<byte> line, HttpResponseMessage response)
                 {
                     unsafe
                     {
-                        fixed (byte* reasonPtr = &reasonBytes.DangerousGetPinnableReference())
+                        try
                         {
-                            response.ReasonPhrase = Encoding.ASCII.GetString(reasonPtr, reasonBytes.Length);
+                            fixed (byte* reasonPtr = &reasonBytes.DangerousGetPinnableReference())
+                            {
+                                response.ReasonPhrase = Encoding.ASCII.GetString(reasonPtr, reasonBytes.Length);
+                            }
+                        }
+                        catch (FormatException e)
+                        {
+                            ThrowInvalidHttpResponse(e);
                         }
                     }
                 }
@@ -1156,6 +1163,8 @@ private static bool EqualsOrdinal(string left, Span<byte> right)
 
         private static void ThrowInvalidHttpResponse() => throw new HttpRequestException(SR.net_http_invalid_response);
 
+        private static void ThrowInvalidHttpResponse(Exception innerException) => throw new HttpRequestException(SR.net_http_invalid_response, innerException);
+
         internal void Trace(string message, [CallerMemberName] string memberName = null) =>
             NetEventSource.Log.HandlerMessage(
                 _pool?.GetHashCode() ?? 0,    // pool ID

src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs

@@ -1280,34 +1280,6 @@ await LoopbackServer.CreateServerAsync(async (socket3, url3) =>
             });
         }
 
-        [OuterLoop] // TODO: Issue #11345
-        [Theory]
-        [InlineData(200)]
-        [InlineData(500)]
-        [InlineData(600)]
-        [InlineData(900)]
-        [InlineData(999)]
-        public async Task GetAsync_ExpectedStatusCode(int statusCode)
-        {
-            await LoopbackServer.CreateServerAsync(async (server, url) =>
-            {
-                using (HttpClient client = CreateHttpClient())
-                {
-                    Task<HttpResponseMessage> getResponseTask = client.GetAsync(url);
-                    await TestHelper.WhenAllCompletedOrAnyFailed(
-                        getResponseTask,
-                        LoopbackServer.ReadRequestAndSendResponseAsync(server,
-                            $"HTTP/1.1 {statusCode}\r\n" +
-                            $"Date: {DateTimeOffset.UtcNow:R}\r\n" +
-                            "\r\n"));
-                    using (HttpResponseMessage response = await getResponseTask)
-                    {
-                        Assert.Equal(statusCode, (int)response.StatusCode);
-                    }
-                }
-            });
-        }
-
         [OuterLoop] // TODO: Issue #11345
         [Theory]
         [InlineData(99)]

src/System.Net.Http/tests/FunctionalTests/HttpProtocolTests.cs

@@ -0,0 +1,143 @@
+// 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.Net.Test.Common;
+using System.Threading;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace System.Net.Http.Functional.Tests
+{
+    public class HttpProtocolTests : HttpClientTest
+    {
+        protected virtual Stream GetStream(Stream s) => s;
+
+        [Theory]
+        [InlineData("HTTP/1.1 200 OK", 200, "OK")]
+        [InlineData("HTTP/1.1 200 Sure why not?", 200, "Sure why not?")]
+        [InlineData("HTTP/1.1 200 OK\x0080", 200, "OK?")]
+        [InlineData("HTTP/1.1 200 O K", 200, "O K")]
+        [InlineData("HTTP/1.1 201 Created", 201, "Created")]
+        [InlineData("HTTP/1.1 202 Accepted", 202, "Accepted")]
+        [InlineData("HTTP/1.1 299 This is not a real status code", 299, "This is not a real status code")]
+        [InlineData("HTTP/1.1 345 redirect to nowhere", 345, "redirect to nowhere")]
+        [InlineData("HTTP/1.1 400 Bad Request", 400, "Bad Request")]
+        [InlineData("HTTP/1.1 500 Internal Server Error", 500, "Internal Server Error")]
+        [InlineData("HTTP/1.1 555 we just don't like you", 555, "we just don't like you")]
+        [InlineData("HTTP/1.1 600 still valid", 600, "still valid")]
+        // TODO #24713: The following pass on Windows on .NET Core but fail on .NET Framework.
+        //[InlineData("HTTP/1.1 200      ", 200, "")]
+        //[InlineData("HTTP/1.1 200      Something", 200, "Something")]
+        //[InlineData("HTTP/1.1\t200 OK", 200, "OK")]
+        //[InlineData("HTTP/1.1 200\tOK", 200, "OK")]
+        //[InlineData("HTTP/1.1 200", 200, "")]
+        //[InlineData("HTTP/1.1 200\t", 200, "")]
+        //[InlineData("HTTP/1.1 200 O\tK", 200, "O\tK")]
+        //[InlineData("HTTP/1.1 200 O    \t\t  \t\t\t\t  \t K", 200, "O    \t\t  \t\t\t\t  \t K")]
+        //[InlineData("HTTP/1.1 999 this\ttoo\t", 999, "this\ttoo\t")]
+        public async Task GetAsync_ExpectedStatusCodeAndReason_Success(string statusLine, int expectedStatusCode, string expectedReason)
+        {
+            await LoopbackServer.CreateServerAsync(async (server, url) =>
+            {
+                using (HttpClient client = CreateHttpClient())
+                {
+                    Task<HttpResponseMessage> getResponseTask = client.GetAsync(url);
+                    await TestHelper.WhenAllCompletedOrAnyFailed(
+                        getResponseTask,
+                        LoopbackServer.ReadRequestAndSendResponseAsync(server,
+                            $"{statusLine}\r\n" +
+                            $"Date: {DateTimeOffset.UtcNow:R}\r\n" +
+                            "\r\n",
+                            new LoopbackServer.Options { ResponseStreamWrapper = GetStream }));
+                    using (HttpResponseMessage response = await getResponseTask)
+                    {
+                        Assert.Equal(expectedStatusCode, (int)response.StatusCode);
+                        Assert.Equal(expectedReason, response.ReasonPhrase);
+                    }
+                }
+            });
+        }
+
+        [Theory]
+        [InlineData("HTTP/1.1 2345")]
+        [InlineData("HTTP/A.1 200 OK")]
+        [InlineData("HTTP/X.Y.Z 200 OK")]
+        // TODO #24713: The following pass on Windows on .NET Core but fail on .NET Framework.
+        //[InlineData("HTTP/0.1 200 OK")]
+        //[InlineData("HTTP/3.5 200 OK")]
+        //[InlineData("HTTP/1.12 200 OK")]
+        //[InlineData("HTTP/12.1 200 OK")]
+        // TODO #24713: The following pass on Windows on .NET Core but fail on UWP / WinRT.
+        //[InlineData("HTTP/1.1 200 O\nK")]
+        //[InlineData("HTTP/1.1 200OK")]
+        //[InlineData("HTTP/1.1 20c")]
+        //[InlineData("HTTP/1.1 23")]
+        //[InlineData("HTTP/1.1 2bc")]
+        // TODO #24713: The following pass on Windows but fail on CurlHandler on Linux.
+        //[InlineData("NOTHTTP/1.1")]
+        //[InlineData("HTTP/1.A 200 OK")]
+        //[InlineData("HTTP 1.1 200 OK")]
+        //[InlineData("ABCD/1.1 200 OK")]
+        //[InlineData("HTTP/1.1")]
+        //[InlineData("HTTP\\1.1 200 OK")]
+        //[InlineData("HTTP/1.1 ")]
+        //[InlineData("HTTP/1.1 !11")]
+        //[InlineData("HTTP/1.1 a11")]
+        //[InlineData("HTTP/1.1 abc")]
+        //[InlineData("HTTP/1.1 200 O\rK")]
+        //[InlineData("HTTP/1.1\t\t")]
+        //[InlineData("HTTP/1.1\t")]
+        //[InlineData("HTTP/1.1  ")]
+        //[InlineData("NOTHTTP/1.1 200 OK")]
+        public async Task GetAsync_InvalidStatusLine_ThrowsException(string responseString)
+        {
+            await LoopbackServer.CreateServerAsync(async (server, url) =>
+            {
+                using (HttpClient client = CreateHttpClient())
+                {
+                    Task ignoredServerTask = LoopbackServer.ReadRequestAndSendResponseAsync(
+                        server,
+                        responseString + "\r\nContent-Length: 0\r\n\r\n",
+                        new LoopbackServer.Options { ResponseStreamWrapper = GetStream });
+
+                    await Assert.ThrowsAsync<HttpRequestException>(() => client.GetAsync(url));
+                }
+            });
+        }
+    }
+
+    public class HttpProtocolTests_Dribble : HttpProtocolTests, IDisposable
+    {
+        protected override Stream GetStream(Stream s) => new DribbleStream(s);
+
+        private sealed class DribbleStream : Stream
+        {
+            private readonly Stream _wrapped;
+
+            public DribbleStream(Stream wrapped) => _wrapped = wrapped;
+
+            public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
+            {
+                for (int i = 0; i < count; i++)
+                {
+                    await _wrapped.WriteAsync(buffer, offset + i, 1);
+                    await Task.Yield(); // introduce short delays, enough to send packets individually but so long as to extend test duration significantly
+                }
+            }
+
+            public override bool CanRead => false;
+            public override bool CanSeek => false;
+            public override bool CanWrite => _wrapped.CanWrite;
+            public override long Length => throw new NotSupportedException();
+            public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
+            public override void Flush() => _wrapped.Flush();
+            public override Task FlushAsync(CancellationToken cancellationToken) => _wrapped.FlushAsync(cancellationToken);
+            public override int Read(byte[] buffer, int offset, int count) => throw new NotImplementedException();
+            public override long Seek(long offset, SeekOrigin origin) => throw new NotImplementedException();
+            public override void SetLength(long value) => throw new NotImplementedException();
+            public override void Write(byte[] buffer, int offset, int count) => throw new NotImplementedException();
+        }
+    }
+}

src/System.Net.Http/tests/FunctionalTests/ManagedHandlerTest.cs

@@ -11,94 +11,94 @@
 using Xunit;
 using Xunit.Abstractions;
 
-// NOTE:
-// Currently the managed handler is opt-in on both Windows and Unix, due to still being a nascent implementation
-// that's missing features, robustness, perf, etc.  One opts into it currently by setting an environment variable,
-// which makes it a bit difficult to test.  There are two straightforward ways to test it:
-// - This file contains test classes that derive from the other test classes in the project that create
-//   HttpClient{Handler} instances, and in the ctor sets the env var and in Dispose removes the env var.
-//   That has the effect of running all of those same tests again, but with the managed handler enabled.
-// - By setting the env var prior to running tests, every test will implicitly use the managed handler,
-//   at which point the tests in this file are duplicative and can be commented out.
-
 namespace System.Net.Http.Functional.Tests
 {
-    public sealed class ManagedHandler_HttpClientTest : HttpClientTest, IDisposable
+    public sealed class ManagedHandler_HttpProtocolTests : HttpProtocolTests
+    {
+        protected override bool UseManagedHandler => true;
+    }
+
+    public sealed class ManagedHandler_HttpProtocolTests_Dribble : HttpProtocolTests_Dribble
+    {
+        protected override bool UseManagedHandler => true;
+    }
+
+    public sealed class ManagedHandler_HttpClientTest : HttpClientTest
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_DiagnosticsTest : DiagnosticsTest, IDisposable
+    public sealed class ManagedHandler_DiagnosticsTest : DiagnosticsTest
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientEKUTest : HttpClientEKUTest, IDisposable
+    public sealed class ManagedHandler_HttpClientEKUTest : HttpClientEKUTest
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientHandler_DangerousAcceptAllCertificatesValidator_Test : HttpClientHandler_DangerousAcceptAllCertificatesValidator_Test, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_DangerousAcceptAllCertificatesValidator_Test : HttpClientHandler_DangerousAcceptAllCertificatesValidator_Test
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientHandler_ClientCertificates_Test : HttpClientHandler_ClientCertificates_Test, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_ClientCertificates_Test : HttpClientHandler_ClientCertificates_Test
     {
         public ManagedHandler_HttpClientHandler_ClientCertificates_Test(ITestOutputHelper output) : base(output) { }
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientHandler_DefaultProxyCredentials_Test : HttpClientHandler_DefaultProxyCredentials_Test, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_DefaultProxyCredentials_Test : HttpClientHandler_DefaultProxyCredentials_Test
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientHandler_MaxConnectionsPerServer_Test : HttpClientHandler_MaxConnectionsPerServer_Test, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_MaxConnectionsPerServer_Test : HttpClientHandler_MaxConnectionsPerServer_Test
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientHandler_ServerCertificates_Test : HttpClientHandler_ServerCertificates_Test, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_ServerCertificates_Test : HttpClientHandler_ServerCertificates_Test
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_PostScenarioTest : PostScenarioTest, IDisposable
+    public sealed class ManagedHandler_PostScenarioTest : PostScenarioTest
     {
         public ManagedHandler_PostScenarioTest(ITestOutputHelper output) : base(output) { }
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_ResponseStreamTest : ResponseStreamTest, IDisposable
+    public sealed class ManagedHandler_ResponseStreamTest : ResponseStreamTest
     {
         public ManagedHandler_ResponseStreamTest(ITestOutputHelper output) : base(output) { }
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientHandler_SslProtocols_Test : HttpClientHandler_SslProtocols_Test, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_SslProtocols_Test : HttpClientHandler_SslProtocols_Test
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_SchSendAuxRecordHttpTest : SchSendAuxRecordHttpTest, IDisposable
+    public sealed class ManagedHandler_SchSendAuxRecordHttpTest : SchSendAuxRecordHttpTest
     {
         public ManagedHandler_SchSendAuxRecordHttpTest(ITestOutputHelper output) : base(output) { }
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientMiniStress : HttpClientMiniStress, IDisposable
+    public sealed class ManagedHandler_HttpClientMiniStress : HttpClientMiniStress
     {
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_HttpClientHandlerTest : HttpClientHandlerTest, IDisposable
+    public sealed class ManagedHandler_HttpClientHandlerTest : HttpClientHandlerTest
     {
         public ManagedHandler_HttpClientHandlerTest(ITestOutputHelper output) : base(output) { }
         protected override bool UseManagedHandler => true;
     }
 
-    public sealed class ManagedHandler_DefaultCredentialsTest : DefaultCredentialsTest, IDisposable
+    public sealed class ManagedHandler_DefaultCredentialsTest : DefaultCredentialsTest
     {
         public ManagedHandler_DefaultCredentialsTest(ITestOutputHelper output) : base(output) { }
         protected override bool UseManagedHandler => true;
@@ -109,19 +109,19 @@ public ManagedHandler_DefaultCredentialsTest(ITestOutputHelper output) : base(ou
     // "cancelable", meaning that the underlying operation will still be running even though we've returned "canceled",
     // or we need to just recognize that cancellation in such situations can be left up to the caller to do the
     // same thing if it's really important.
-    //public sealed class ManagedHandler_CancellationTest : CancellationTest, IDisposable
+    //public sealed class ManagedHandler_CancellationTest : CancellationTest
     //{
     //    public ManagedHandler_CancellationTest(ITestOutputHelper output) : base(output) { }
     //    protected override bool UseManagedHandler => true;
     //}
 
     // TODO #23142: The managed handler doesn't currently track how much data was written for the response headers.
-    //public sealed class ManagedHandler_HttpClientHandler_MaxResponseHeadersLength_Test : HttpClientHandler_MaxResponseHeadersLength_Test, IDisposable
+    //public sealed class ManagedHandler_HttpClientHandler_MaxResponseHeadersLength_Test : HttpClientHandler_MaxResponseHeadersLength_Test
     //{
     //    protected override bool UseManagedHandler => true;
     //}
 
-    public sealed class ManagedHandler_HttpClientHandler_DuplexCommunication_Test : HttpClientTestBase, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_DuplexCommunication_Test : HttpClientTestBase
     {
         protected override bool UseManagedHandler => true;
 
@@ -253,7 +253,7 @@ public override void Flush() { }
 
     }
 
-    public sealed class ManagedHandler_HttpClientHandler_ConnectionPooling_Test : HttpClientTestBase, IDisposable
+    public sealed class ManagedHandler_HttpClientHandler_ConnectionPooling_Test : HttpClientTestBase
     {
         protected override bool UseManagedHandler => true;
 

src/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj

@@ -89,6 +89,7 @@
     <Compile Include="HttpContentTest.cs" />
     <Compile Include="HttpMessageInvokerTest.cs" />
     <Compile Include="HttpMethodTest.cs" />
+    <Compile Include="HttpProtocolTests.cs" />
     <Compile Include="HttpRequestMessageTest.cs" />
     <Compile Include="HttpResponseMessageTest.cs" />
     <Compile Include="LoopbackGetRequestHttpProxy.cs" />