reject invalid pseudo-headers (dotnet/corefx#37069)

* reject invalid pseudo-headers

* feedback from review

* feedback from review

* fix typo in comment

* feedback from review

* syncup with recent changes

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

diff --git a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs
index 5b527c6..c036304 100644 (file)
--- a/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs
+++ b/src/libraries/Common/tests/System/Net/Http/Http2LoopbackServer.cs
@@ -115,8 +115,17 @@ namespace System.Net.Test.Common
 
             // First read the frame headers, which should tell us how long the rest of the frame is.
             byte[] headerBytes = new byte[Frame.FrameHeaderLength];
-            if (!await FillBufferAsync(headerBytes, timeoutCts.Token).ConfigureAwait(false))
+
+            try
+            {
+                if (!await FillBufferAsync(headerBytes, timeoutCts.Token).ConfigureAwait(false))
+                {
+                    return null;
+                }
+            }
+            catch (IOException)
             {
+                // eat errors when client aborts connection and return null.
                 return null;
             }
 

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
index 1a45dca..2211f84 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs
@@ -36,7 +36,7 @@ namespace System.Net.Http
             private readonly int _streamId;
             private readonly CreditManager _streamWindow;
             private readonly HttpRequestMessage _request;
-            private readonly HttpResponseMessage _response;
+            private HttpResponseMessage _response;
 
             private ArrayBuffer _responseBuffer; // mutable struct, do not make this readonly
             private int _pendingWindowUpdate;
@@ -66,12 +66,6 @@ namespace System.Net.Http
                 _state = StreamState.ExpectingHeaders;
 
                 _request = request;
-                _response = new HttpResponseMessage()
-                {
-                    Version = HttpVersion.Version20,
-                    RequestMessage = request,
-                    Content = new HttpConnectionResponseContent()
-                };
 
                 _disposed = false;
 
@@ -121,6 +115,7 @@ namespace System.Net.Http
 
             public void OnResponseHeader(ReadOnlySpan<byte> name, ReadOnlySpan<byte> value)
             {
+                Debug.Assert(name != null && name.Length > 0);
                 // TODO: ISSUE 31309: Optimize HPACK static table decoding
 
                 lock (SyncObject)
@@ -130,7 +125,7 @@ namespace System.Net.Http
                         throw new Http2ProtocolException(Http2ProtocolErrorCode.ProtocolError);
                     }
 
-                    if (name.SequenceEqual(s_statusHeaderName))
+                    if (name[0] == (byte)':')
                     {
                         if (_state == StreamState.ExpectingTrailingHeaders)
                         {
@@ -139,19 +134,46 @@ namespace System.Net.Http
                             throw new HttpRequestException(SR.net_http_invalid_response_pseudo_header_in_trailer);
                         }
 
-                        byte status1, status2, status3;
-                        if (value.Length != 3 ||
-                            !IsDigit(status1 = value[0]) ||
-                            !IsDigit(status2 = value[1]) ||
-                            !IsDigit(status3 = value[2]))
+                        if (name.SequenceEqual(s_statusHeaderName))
                         {
-                            throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_code, Encoding.ASCII.GetString(value)));
+                            if (_response != null)
+                            {
+                                if (NetEventSource.IsEnabled) _connection.Trace("Received duplicate status headers.");
+                                throw new Http2ProtocolException(Http2ProtocolErrorCode.ProtocolError);
+                            }
+
+                            byte status1, status2, status3;
+                            if (value.Length != 3 ||
+                                !IsDigit(status1 = value[0]) ||
+                                !IsDigit(status2 = value[1]) ||
+                                !IsDigit(status3 = value[2]))
+                            {
+                                throw new HttpRequestException(SR.Format(SR.net_http_invalid_response_status_code, Encoding.ASCII.GetString(value)));
+                            }
+
+                            int statusValue = (100 * (status1 - '0') + 10 * (status2 - '0') + (status3 - '0'));
+                            _response = new HttpResponseMessage()
+                            {
+                                Version = HttpVersion.Version20,
+                                RequestMessage = _request,
+                                Content = new HttpConnectionResponseContent(),
+                                StatusCode = (HttpStatusCode)statusValue
+                            };
+                        }
+                        else
+                        {
+                            if (NetEventSource.IsEnabled) _connection.Trace("Invalid response pseudo-header '{System.Text.Encoding.ASCII.GetString(name)}'.");
+                            throw new HttpRequestException(SR.net_http_invalid_response);
                         }
-
-                        _response.SetStatusCodeWithoutValidation((HttpStatusCode)(100 * (status1 - '0') + 10 * (status2 - '0') + (status3 - '0')));
                     }
                     else
                     {
+                        if (_response == null)
+                        {
+                            if (NetEventSource.IsEnabled) _connection.Trace($"Received header before status pseudo-header.");
+                            throw new HttpRequestException(SR.net_http_invalid_response);
+                        }
+
                         if (!HeaderDescriptor.TryGet(name, out HeaderDescriptor descriptor))
                         {
                             // Invalid header name

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 b85cc03..9669d6f 100644 (file)
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
@@ -6,6 +6,7 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
 using System.Net.Test.Common;
+using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -1306,5 +1307,60 @@ namespace System.Net.Http.Functional.Tests
                 await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await clientTask);
             }
         }
+
+        // rfc7540 8.1.2.3.
+        [ConditionalFact(nameof(SupportsAlpn))]
+        public async Task Http2GetAsync_MultipleStatusHeaders_Throws()
+        {
+            using (var server = Http2LoopbackServer.CreateServer())
+            using (HttpClient client = CreateHttpClient())
+            {
+                IList<HttpHeaderData> headers = new HttpHeaderData[] { new HttpHeaderData(":status", "300"), new HttpHeaderData("x-test", "Http2GetAsync_MultipleStatusHeaders_Throws") };
+                Task<HttpResponseMessage> sendTask = client.GetAsync(server.Address);
+
+                await server.EstablishConnectionAsync();
+                int streamId = await server.ReadRequestHeaderAsync();
+                await server.SendResponseHeadersAsync(streamId, endStream : true, headers: headers);
+                await Assert.ThrowsAsync<HttpRequestException>(() => sendTask);
+            }
+        }
+
+        // rfc7540 8.1.2.3.
+        [ConditionalFact(nameof(SupportsAlpn))]
+        public async Task Http2GetAsync_StatusHeaderNotFirst_Throws()
+        {
+            using (var server = Http2LoopbackServer.CreateServer())
+            using (HttpClient client = CreateHttpClient())
+            {
+                IList<HttpHeaderData> headers = new HttpHeaderData[] { new HttpHeaderData("x-test", "Http2GetAsync_StatusHeaderNotFirst_Throws"), new HttpHeaderData(":status", "200") };
+                Task<HttpResponseMessage> sendTask = client.GetAsync(server.Address);
+
+                await server.EstablishConnectionAsync();
+                int streamId = await server.ReadRequestHeaderAsync();
+                await server.SendResponseHeadersAsync(streamId, endStream : true, isTrailingHeader : true, headers: headers);
+
+                await Assert.ThrowsAsync<HttpRequestException>(() => sendTask);
+            }
+        }
+
+        // rfc7540 8.1.2.3.
+        [ConditionalFact(nameof(SupportsAlpn))]
+        public async Task Http2GetAsync_TrailigPseudo_Throw()
+        {
+            using (var server = Http2LoopbackServer.CreateServer())
+            using (HttpClient client = CreateHttpClient())
+            {
+                IList<HttpHeaderData> headers = new HttpHeaderData[] { new HttpHeaderData(":path", "http"), new HttpHeaderData("x-test", "Http2GetAsync_TrailigPseudo_Throw") };
+                Task<HttpResponseMessage> sendTask = client.GetAsync(server.Address);
+
+                await server.EstablishConnectionAsync();
+                int streamId = await server.ReadRequestHeaderAsync();
+                await server.SendDefaultResponseHeadersAsync(streamId);
+                await server.SendResponseDataAsync(streamId, Encoding.ASCII.GetBytes("hello"), endStream: false);
+                await server.SendResponseHeadersAsync(streamId, endStream : true, isTrailingHeader : true, headers: headers);
+
+                await Assert.ThrowsAsync<HttpRequestException>(() => sendTask);
+            }
+        }
     }
 }