Implement MaxResponseHeadersLength on ManagedHandler (#26906)

Author: stephentoub

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

@@ -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.Buffers.Text;
 using System.Diagnostics;
 using System.IO;
 using System.Threading;
@@ -13,81 +14,83 @@ internal sealed partial class HttpConnection
     {
         private sealed class ChunkedEncodingReadStream : HttpContentReadStream
         {
+            /// <summary>How long a chunk indicator is allowed to be.</summary>
+            /// <remarks>
+            /// While most chunks indicators will contain no more than ulong.MaxValue.ToString("X").Length characters,
+            /// "chunk extensions" are allowed. We place a limit on how long a line can be to avoid OOM issues if an
+            /// infinite chunk length is sent.  This value is arbitrary and can be changed as needed.
+            /// </remarks>
+            private const int MaxChunkBytesAllowed = 16*1024;
+            /// <summary>How long a trailing header can be.  This value is arbitrary and can be changed as needed.</summary>
+            private const int MaxTrailingHeaderLength = 16*1024;
+            /// <summary>The number of bytes remaining in the chunk.</summary>
             private ulong _chunkBytesRemaining;
 
-            public ChunkedEncodingReadStream(HttpConnection connection)
-                : base(connection)
+            public ChunkedEncodingReadStream(HttpConnection connection) : base(connection)
             {
-                _chunkBytesRemaining = 0;
             }
 
-            private async Task<bool> TryGetNextChunk(CancellationToken cancellationToken)
+            private async Task<bool> TryGetNextChunkAsync(CancellationToken cancellationToken)
             {
                 Debug.Assert(_chunkBytesRemaining == 0);
 
-                // Start of chunk, read chunk size.
-                ulong chunkSize = ParseHexSize(await _connection.ReadNextLineAsync(cancellationToken).ConfigureAwait(false));
-                _chunkBytesRemaining = chunkSize;
-
-                if (chunkSize > 0)
-                {
-                    return true;
-                }
-
-                // We received a chunk size of 0, which indicates end of response body. 
-                // Read and discard any trailing headers, until we see an empty line.
-                while (!LineIsEmpty(await _connection.ReadNextLineAsync(cancellationToken).ConfigureAwait(false)))
-                    ;
+                // Read the start of the chunk line.
+                _connection._allowedReadLineBytes = MaxChunkBytesAllowed;
+                ArraySegment<byte> line = await _connection.ReadNextLineAsync(cancellationToken).ConfigureAwait(false);
 
-                _connection.ReturnConnectionToPool();
-                _connection = null;
-                return false;
-            }
-
-            private ulong ParseHexSize(ArraySegment<byte> line)
-            {
-                if (line.Count == 0)
+                // Parse the hex value.
+                if (!Utf8Parser.TryParse(line.AsReadOnlySpan(), out ulong chunkSize, out int bytesConsumed, 'X'))
                 {
                     throw new IOException(SR.net_http_invalid_response);
                 }
-
-                ulong size = 0;
-                try
+                else if (bytesConsumed != line.Count)
                 {
-                    for (int i = 0; i < line.Count; i++)
+                    // There was data after the chunk size, presumably a "chunk extension".
+                    // Allow tabs and spaces and then stop validating once we get to an extension.
+                    int offset = line.Offset + bytesConsumed, end = line.Count - bytesConsumed + line.Offset;
+                    for (int i = offset; i < end; i++)
                     {
-                        char c = (char)line[i];
-                        if ((uint)(c - '0') <= '9' - '0')
-                        {
-                            size = checked(size * 16 + ((ulong)c - '0'));
-                        }
-                        else if ((uint)(c - 'a') <= ('f' - 'a'))
+                        char c = (char)line.Array[i];
+                        if (c == ';')
                         {
-                            size = checked(size * 16 + ((ulong)c - 'a' + 10));
+                            break;
                         }
-                        else if ((uint)(c - 'A') <= ('F' - 'A'))
-                        {
-                            size = checked(size * 16 + ((ulong)c - 'A' + 10));
-                        }
-                        else
+                        else if (c != ' ' && c != '\t') // not called out in the RFC, but WinHTTP allows it
                         {
                             throw new IOException(SR.net_http_invalid_response);
                         }
                     }
                 }
-                catch (OverflowException e)
+
+                _chunkBytesRemaining = chunkSize;
+                if (chunkSize > 0)
                 {
-                    throw new IOException(SR.net_http_invalid_response, e);
+                    return true;
                 }
-                return size;
+
+                // We received a chunk size of 0, which indicates end of response body. 
+                // Read and discard any trailing headers, until we see an empty line.
+                while (true)
+                {
+                    _connection._allowedReadLineBytes = MaxTrailingHeaderLength;
+                    if (LineIsEmpty(await _connection.ReadNextLineAsync(cancellationToken).ConfigureAwait(false)))
+                    {
+                        break;
+                    }
+                }
+
+                _connection.ReturnConnectionToPool();
+                _connection = null;
+                return false;
             }
 
-            private async Task ConsumeChunkBytes(ulong bytesConsumed, CancellationToken cancellationToken)
+            private async Task ConsumeChunkBytesAsync(ulong bytesConsumed, CancellationToken cancellationToken)
             {
                 Debug.Assert(bytesConsumed <= _chunkBytesRemaining);
                 _chunkBytesRemaining -= bytesConsumed;
                 if (_chunkBytesRemaining == 0)
                 {
+                    _connection._allowedReadLineBytes = 2; // \r\n
                     if (!LineIsEmpty(await _connection.ReadNextLineAsync(cancellationToken).ConfigureAwait(false)))
                     {
                         ThrowInvalidHttpResponse();
@@ -111,7 +114,7 @@ public override async ValueTask<int> ReadAsync(Memory<byte> destination, Cancell
 
                 if (_chunkBytesRemaining == 0)
                 {
-                    if (!await TryGetNextChunk(cancellationToken).ConfigureAwait(false))
+                    if (!await TryGetNextChunkAsync(cancellationToken).ConfigureAwait(false))
                     {
                         // End of response body
                         return 0;
@@ -131,7 +134,7 @@ public override async ValueTask<int> ReadAsync(Memory<byte> destination, Cancell
                     throw new IOException(SR.net_http_invalid_response);
                 }
 
-                await ConsumeChunkBytes((ulong)bytesRead, cancellationToken).ConfigureAwait(false);
+                await ConsumeChunkBytesAsync((ulong)bytesRead, cancellationToken).ConfigureAwait(false);
 
                 return bytesRead;
             }
@@ -152,13 +155,13 @@ public override async Task CopyToAsync(Stream destination, int bufferSize, Cance
                 if (_chunkBytesRemaining > 0)
                 {
                     await _connection.CopyToAsync(destination, _chunkBytesRemaining, cancellationToken).ConfigureAwait(false);
-                    await ConsumeChunkBytes(_chunkBytesRemaining, cancellationToken).ConfigureAwait(false);
+                    await ConsumeChunkBytesAsync(_chunkBytesRemaining, cancellationToken).ConfigureAwait(false);
                 }
 
-                while (await TryGetNextChunk(cancellationToken).ConfigureAwait(false))
+                while (await TryGetNextChunkAsync(cancellationToken).ConfigureAwait(false))
                 {
                     await _connection.CopyToAsync(destination, _chunkBytesRemaining, cancellationToken).ConfigureAwait(false);
-                    await ConsumeChunkBytes(_chunkBytesRemaining, cancellationToken).ConfigureAwait(false);
+                    await ConsumeChunkBytesAsync(_chunkBytesRemaining, cancellationToken).ConfigureAwait(false);
                 }
             }
         }

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

@@ -56,6 +56,7 @@ internal sealed partial class HttpConnection : IDisposable
         private readonly byte[] _writeBuffer;
         private int _writeOffset;
         private Exception _pendingException;
+        private int _allowedReadLineBytes;
 
         private Task<int> _readAheadTask;
         private byte[] _readBuffer;
@@ -226,7 +227,7 @@ public async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, Can
 
                 await WriteStringAsync(request.RequestUri.PathAndQuery, cancellationToken).ConfigureAwait(false);
 
-                // fall-back to 1.1 for all versions other than 1.0
+                // Fall back to 1.1 for all versions other than 1.0
                 Debug.Assert(request.Version.Major >= 0 && request.Version.Minor >= 0); // guaranteed by Version class
                 bool isHttp10 = request.Version.Minor == 0 && request.Version.Major == 1;
                 await WriteBytesAsync(isHttp10 ? s_spaceHttp10NewlineAsciiBytes : s_spaceHttp11NewlineAsciiBytes,
@@ -313,6 +314,7 @@ await WriteBytesAsync(isHttp10 ? s_spaceHttp10NewlineAsciiBytes : s_spaceHttp11N
                 }
 
                 // Start to read response.
+                _allowedReadLineBytes = _pool.Pools.Settings._maxResponseHeadersLength * 1024;
 
                 // We should not have any buffered data here; if there was, it should have been treated as an error
                 // by the previous request handling.  (Note we do not support HTTP pipelining.)
@@ -378,6 +380,7 @@ await WriteBytesAsync(isHttp10 ? s_spaceHttp10NewlineAsciiBytes : s_spaceHttp11N
                             {
                                 ThrowInvalidHttpResponse();
                             }
+
                             ParseStatusLine(await ReadNextLineAsync(cancellationToken).ConfigureAwait(false), response);
                         }
                     }
@@ -474,10 +477,7 @@ await WriteBytesAsync(isHttp10 ? s_spaceHttp10NewlineAsciiBytes : s_spaceHttp11N
             }
         }
 
-        private static bool LineIsEmpty(ArraySegment<byte> line)
-        {
-            return line.Count == 0;
-        }
+        private static bool LineIsEmpty(ArraySegment<byte> line) => line.Count == 0;
 
         private async Task SendRequestContentAsync(HttpRequestMessage request, HttpContentWriteStream stream)
         {
@@ -896,6 +896,11 @@ private async ValueTask<ArraySegment<byte>> ReadNextLineAsync(CancellationToken
                     }
 
                     // Advance read position past the LF
+                    _allowedReadLineBytes -= lfIndex + 1 - scanOffset;
+                    if (_allowedReadLineBytes < 0)
+                    {
+                        ThrowInvalidHttpResponse();
+                    }
                     _readOffset = lfIndex + 1;
 
                     return new ArraySegment<byte>(_readBuffer, startIndex, length);
@@ -904,6 +909,11 @@ private async ValueTask<ArraySegment<byte>> ReadNextLineAsync(CancellationToken
                 // Couldn't find LF.  Read more.
                 // Note this may cause _readOffset to change.
                 previouslyScannedBytes = _readLength - _readOffset;
+                _allowedReadLineBytes -= _readLength - scanOffset;
+                if (_allowedReadLineBytes < 0)
+                {
+                    ThrowInvalidHttpResponse();
+                }
                 await FillAsync(cancellationToken).ConfigureAwait(false);
             }
         }