improve error handling on failed Http/2 handshake (dotnet/corefx#37050)

* improve error handling on failed Http/2 handshake

* remove extra space

* add test for HTTP2 client talking to HTTP1 server.

* fix broken tests

* improve exception handling

* update exception handling

* ws update

* fix bad merge

* update test and changes from dotnet/corefx#37223

* use _abortException only if the stream is not aborted already

* fix IsAborted check

* updates to syncup with recent changes

Commit migrated from https://github.com/dotnet/corefx/commit/c412fa7dc60559dc38050c63b2e34b5156a2499e

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
index 029185f..2bf95d8 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
@@ -45,6 +45,7 @@ namespace System.Net.Http
         private int _pendingWriters;
 
         private bool _disposed;
+        private Exception _abortException;
 
         // If an in-progress write is canceled we need to be able to immediately
         // report a cancellation to the user, but also block the connection until
@@ -173,17 +174,24 @@ namespace System.Net.Http
             }
         }
 
-        private async ValueTask<FrameHeader> ReadFrameAsync()
+        private async ValueTask<FrameHeader> ReadFrameAsync(bool initialFrame = false)
         {
             // Read frame header
             await EnsureIncomingBytesAsync(FrameHeader.Size).ConfigureAwait(false);
             FrameHeader frameHeader = FrameHeader.ReadFrom(_incomingBuffer.ActiveSpan);
-            _incomingBuffer.Discard(FrameHeader.Size);
 
             if (frameHeader.Length > FrameHeader.MaxLength)
             {
-                throw new Http2ProtocolException(Http2ProtocolErrorCode.FrameSizeError);
+                if (initialFrame && NetEventSource.IsEnabled)
+                {
+                    string response = System.Text.Encoding.ASCII.GetString(_incomingBuffer.ActiveSpan.Slice(0, Math.Min(20, _incomingBuffer.ActiveSpan.Length)));
+                    Trace($"HTTP/2 handshake failed. Server returned {response}");
+                }
+
+                _incomingBuffer.Discard(FrameHeader.Size);
+                throw new Http2ProtocolException(initialFrame ? Http2ProtocolErrorCode.ProtocolError : Http2ProtocolErrorCode.FrameSizeError);
             }
+            _incomingBuffer.Discard(FrameHeader.Size);
 
             // Read frame contents
             await EnsureIncomingBytesAsync(frameHeader.Length).ConfigureAwait(false);
@@ -195,14 +203,13 @@ namespace System.Net.Http
         {
             try
             {
-                // Receive the initial SETTINGS frame from the peer.
-                FrameHeader frameHeader = await ReadFrameAsync().ConfigureAwait(false);
+                FrameHeader frameHeader = await ReadFrameAsync(initialFrame: true).ConfigureAwait(false);
                 if (frameHeader.Type != FrameType.Settings || frameHeader.AckFlag)
                 {
                     throw new Http2ProtocolException(Http2ProtocolErrorCode.ProtocolError);
                 }
 
-                // Process the SETTINGS frame.  This will send an ACK.
+                // Process the initial SETTINGS frame. This will send an ACK.
                 ProcessSettingsFrame(frameHeader);
 
                 // Keep processing frames as they arrive.
@@ -1117,6 +1124,10 @@ namespace System.Net.Http
         {
             // The connection has failed, e.g. failed IO or a connection-level frame error.
             // Abort all streams and cause further processing to fail.
+            if (!IsAborted())
+            {
+                _abortException = abortException;
+            }
             AbortStreams(0, abortException);
         }
 
@@ -1358,7 +1369,7 @@ namespace System.Net.Http
                     e is ObjectDisposedException ||
                     e is Http2ProtocolException)
                 {
-                    replacementException = new HttpRequestException(SR.net_http_client_execution_error, e);
+                    replacementException = new HttpRequestException(SR.net_http_client_execution_error, _abortException ?? e);
                 }
                 else if (e is OperationCanceledException oce)
                 {

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
index 9669d6f..b45dbca 100644 (file)
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
@@ -4,6 +4,7 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Net.Security;
 using System.Linq;
 using System.Net.Test.Common;
 using System.Text;
@@ -1308,6 +1309,48 @@ namespace System.Net.Http.Functional.Tests
             }
         }
 
+        [ConditionalFact(nameof(SupportsAlpn))]
+        public async Task Http2_ProtocolMismatch_Throws()
+        {
+            HttpClientHandler handler = CreateHttpClientHandler();
+            handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates;
+
+            using (HttpClient client = CreateHttpClient())
+            {
+                // Create HTTP/1.1 loopback server and advertise HTTP2 via ALPN.
+                await LoopbackServer.CreateServerAsync(async (server, uri) =>
+                {
+                    // Convert http to https as we are going to negotiate TLS.
+                    Task<string> requestTask = client.GetStringAsync(uri.ToString().Replace("http://", "https://"));
+
+                    await server.AcceptConnectionAsync(async connection =>
+                    {
+                        // negotiate TLS with ALPN H/2
+                        var sslStream = new SslStream(connection.Stream, false, delegate { return true; });
+                        SslServerAuthenticationOptions options = new SslServerAuthenticationOptions();
+                        options.ServerCertificate = Net.Test.Common.Configuration.Certificates.GetServerCertificate();
+                        options.ApplicationProtocols = new List<SslApplicationProtocol>() { SslApplicationProtocol.Http2 };
+                        options.ApplicationProtocols.Add(SslApplicationProtocol.Http2);
+                        // Negotiate TLS.
+                        await sslStream.AuthenticateAsServerAsync(options, CancellationToken.None).ConfigureAwait(false);
+                        // Send back HTTP/1.1 response
+                        await sslStream.WriteAsync(Encoding.ASCII.GetBytes("HTTP/1.1 400 Unrecognized request\r\n\r\n"), CancellationToken.None);
+                    });
+
+                    try {
+                        await requestTask;
+                        throw new Exception("Should not be here");
+                    }
+                    catch (HttpRequestException e)
+                    {
+                        Assert.NotNull(e.InnerException);
+                        // TBD expect Http2ProtocolException when/if exposed
+                        Assert.False(e.InnerException is ObjectDisposedException);
+                    }
+                    //});
+                });
+        }
+        
         // rfc7540 8.1.2.3.
         [ConditionalFact(nameof(SupportsAlpn))]
         public async Task Http2GetAsync_MultipleStatusHeaders_Throws()