support LF line endings and trailer headers

Author: Geoff Kizer

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

@@ -34,15 +34,23 @@ private async Task<bool> TryGetNextChunk(CancellationToken cancellationToken)
                     return true;
                 }
 
-                // Indicates end of response body. We expect final CRLF after this.
-                await _connection.ReadCrLfAsync(cancellationToken).ConfigureAwait(false);
+                // 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)))
+                    ;
+
                 _connection.ReturnConnectionToPool();
                 _connection = null;
                 return false;
             }
 
             private ulong ParseHexSize(ArraySegment<byte> line)
             {
+                if (line.Count == 0)
+                {
+                    throw new IOException(SR.net_http_invalid_response);
+                }
+
                 ulong size = 0;
                 try
                 {
@@ -63,10 +71,6 @@ private ulong ParseHexSize(ArraySegment<byte> line)
                         }
                         else
                         {
-                            if (c == '\r' && i > 0)
-                            {
-                                break;
-                            }
                             throw new IOException(SR.net_http_invalid_response);
                         }
                     }
@@ -84,7 +88,7 @@ private async Task ConsumeChunkBytes(ulong bytesConsumed, CancellationToken canc
                 _chunkBytesRemaining -= bytesConsumed;
                 if (_chunkBytesRemaining == 0)
                 {
-                    await _connection.ReadCrLfAsync(cancellationToken).ConfigureAwait(false);
+                    await _connection.ReadEmptyLineAsync(cancellationToken).ConfigureAwait(false);
                 }
             }
 

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

@@ -407,12 +407,8 @@ await WriteBytesAsync(isHttp10 ? s_spaceHttp10NewlineAsciiBytes : s_spaceHttp11N
                         // And if this was 100 continue, deal with the extra headers.
                         if (response.StatusCode == HttpStatusCode.Continue)
                         {
-                            // We got our continue header.  Read the subsequent \r\n and parse the additional status line.
-                            if (!LineIsEmpty(await ReadNextLineAsync(cancellationToken).ConfigureAwait(false)))
-                            {
-                                ThrowInvalidHttpResponse();
-                            }
-
+                            // We got our continue header.  Read the subsequent empty line and parse the additional status line.
+                            await ReadEmptyLineAsync(cancellationToken).ConfigureAwait(false);
                             ParseStatusLine(await ReadNextLineAsync(cancellationToken).ConfigureAwait(false), response);
                         }
                     }
@@ -511,8 +507,7 @@ await WriteBytesAsync(isHttp10 ? s_spaceHttp10NewlineAsciiBytes : s_spaceHttp11N
 
         private static bool LineIsEmpty(ArraySegment<byte> line)
         {
-            Debug.Assert(line.Count >= 2, "Lines should always be \r\n terminated.");
-            return line.Count == 2;
+            return line.Count == 0;
         }
 
         private async Task SendRequestContentAsync(Task copyTask, HttpContentWriteStream stream)
@@ -572,7 +567,7 @@ private void ParseStatusLine(Span<byte> line, HttpResponseMessage response)
             // We sent the request version as either 1.0 or 1.1.
             // We expect a response version of the form 1.X, where X is a single digit as per RFC.
 
-            if (line.Length < 14 || // "HTTP/1.1 123\r\n" with optional phrase before the crlf
+            if (line.Length < 12 || // "HTTP/1.1 123" with optional phrase 
                 line[0] != 'H' ||
                 line[1] != 'T' ||
                 line[2] != 'T' ||
@@ -604,39 +599,25 @@ private void ParseStatusLine(Span<byte> line, HttpResponseMessage response)
                 (HttpStatusCode)(100 * (status1 - '0') + 10 * (status2 - '0') + (status3 - '0'));
 
             // Parse (optional) reason phrase
-            byte c = line[12];
-            if (c == '\r')
+            if (line.Length == 12)
             {
                 response.ReasonPhrase = string.Empty;
             }
-            else if (c != ' ')
+            else if (line[12] != ' ')
             {
                 ThrowInvalidHttpResponse();
             }
             else
             {
-                Span<byte> reasonBytes = line.Slice(13, line.Length - 13 - 2); // 2 == \r\n ending trimmed off
+                Span<byte> reasonBytes = line.Slice(13);
                 string knownReasonPhrase = HttpStatusDescription.Get(response.StatusCode);
                 if (knownReasonPhrase != null && EqualsOrdinal(knownReasonPhrase, reasonBytes))
                 {
                     response.ReasonPhrase = knownReasonPhrase;
                 }
                 else
                 {
-                    unsafe
-                    {
-                        try
-                        {
-                            fixed (byte* reasonPtr = &MemoryMarshal.GetReference(reasonBytes))
-                            {
-                                response.ReasonPhrase = Encoding.ASCII.GetString(reasonPtr, reasonBytes.Length);
-                            }
-                        }
-                        catch (FormatException e)
-                        {
-                            ThrowInvalidHttpResponse(e);
-                        }
-                    }
+                    response.ReasonPhrase = Encoding.ASCII.GetString(reasonBytes);
                 }
             }
         }
@@ -649,6 +630,8 @@ private void ParseHeaderNameValue(ArraySegment<byte> line, HttpResponseMessage r
 
         private void ParseHeaderNameValue(Span<byte> line, HttpResponseMessage response)
         {
+            Debug.Assert(line.Length > 0);
+
             int pos = 0;
             while (line[pos] != (byte)':' && line[pos] != (byte)' ')
             {
@@ -666,8 +649,6 @@ private void ParseHeaderNameValue(Span<byte> line, HttpResponseMessage response)
                 return;
             }
 
-            // CONSIDER: trailing whitespace?
-
             if (!HeaderDescriptor.TryGet(line.Slice(0, pos), out HeaderDescriptor descriptor))
             {
                 // Ignore invalid header name
@@ -692,12 +673,12 @@ private void ParseHeaderNameValue(Span<byte> line, HttpResponseMessage response)
             }
 
             // Skip whitespace after colon
-            while (pos < line.Length && (line[pos] == (byte)' ' || line[pos] == '\t'))
+            while (pos < line.Length && (line[pos] == (byte)' ' || line[pos] == (byte)'\t'))
             {
                 pos++;
             }
 
-            string headerValue = descriptor.GetHeaderValue(line.Slice(pos, line.Length - pos - 2));     // trim trailing \r\n
+            string headerValue = descriptor.GetHeaderValue(line.Slice(pos));
 
             // Note we ignore the return value from TryAddWithoutValidation; 
             // if the header can't be added, we silently drop it.
@@ -933,54 +914,37 @@ private Task WriteToStreamAsync(ReadOnlyMemory<byte> source, CancellationToken c
 
         private async ValueTask<ArraySegment<byte>> ReadNextLineAsync(CancellationToken cancellationToken)
         {
-            int searchOffset = 0;
+            int previouslyScannedBytes = 0;
             while (true)
             {
-                int startIndex = _readOffset + searchOffset;
-                int length = _readLength - startIndex;
-                int crPos = Array.IndexOf(_readBuffer, (byte)'\r', startIndex, length);
-                if (crPos < 0)
-                {
-                    // Couldn't find a \r.  Read more.
-                    searchOffset = length;
-                    await FillAsync(cancellationToken).ConfigureAwait(false);
-                }
-                else if (crPos + 1 >= _readLength)
+                int scanOffset = _readOffset + previouslyScannedBytes;
+                int endIndex = Array.IndexOf(_readBuffer, (byte)'\n', scanOffset, _readLength - scanOffset);
+                if (endIndex >= 0)
                 {
-                    // We found a \r, but we don't have enough data buffered to read the \n.
-                    searchOffset = length - 1;
-                    await FillAsync(cancellationToken).ConfigureAwait(false);
-                }
-                else if (_readBuffer[crPos + 1] == '\n')
-                {
-                    // We found a \r\n.  Return the data up to and including it.
-                    int lineLength = crPos - _readOffset + 2;
-                    var result = new ArraySegment<byte>(_readBuffer, _readOffset, lineLength);
-                    _readOffset += lineLength;
-                    return result;
-                }
-                else
-                {
-                    ThrowInvalidHttpResponse();
+                    int startIndex = _readOffset;
+                    _readOffset = endIndex + 1;
+
+                    if (endIndex > startIndex && _readBuffer[endIndex - 1] == '\r')
+                    {
+                        endIndex--;
+                    }
+
+                    return new ArraySegment<byte>(_readBuffer, startIndex, endIndex - startIndex);
                 }
-            }
-        }
 
-        private async Task ReadCrLfAsync(CancellationToken cancellationToken)
-        {
-            while (_readLength - _readOffset < 2)
-            {
-                // We have fewer than 2 chars buffered.  Get more.
+                // Couldn't find LF.  Read more.
+                // Note this may cause _readOffset to change.
+                previouslyScannedBytes = _readLength - _readOffset;
                 await FillAsync(cancellationToken).ConfigureAwait(false);
             }
+        }
 
-            // We expect \r\n.  If so, consume them.  If not, it's an error.
-            if (_readBuffer[_readOffset] != '\r' || _readBuffer[_readOffset + 1] != '\n')
+        private async Task ReadEmptyLineAsync(CancellationToken cancellationToken)
+        {
+            if (!LineIsEmpty(await ReadNextLineAsync(cancellationToken).ConfigureAwait(false)))
             {
                 ThrowInvalidHttpResponse();
             }
-
-            _readOffset += 2;
         }
 
         // Throws IOException on EOF.  This is only called when we expect more data.

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

@@ -1159,15 +1159,15 @@ await TestHelper.WhenAllCompletedOrAnyFailed(
         }
 
         [OuterLoop] // TODO: Issue #11345
-        [ActiveIssue(17174, TestPlatforms.AnyUnix)] // https://github.com/curl/curl/issues/1354
         [Theory]
         [InlineData(false)]
         [InlineData(true)]
         public async Task GetAsync_TrailingHeaders_Ignored(bool includeTrailerHeader)
         {
-            if (UseManagedHandler)
+            if (IsCurlHandler)
             {
-                // TODO #23130: The managed handler isn't correctly handling trailing headers.
+                // ActiveIssue #17174: CurlHandler has a problem here
+                // https://github.com/curl/curl/issues/1354
                 return;
             }
 

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

@@ -31,6 +31,11 @@ static HttpClientTestBase()
 
         protected virtual bool UseManagedHandler => false;
 
+        protected bool IsWinHttpHandler => !UseManagedHandler && PlatformDetection.IsWindows && !PlatformDetection.IsUap && !PlatformDetection.IsFullFramework;
+        protected bool IsCurlHandler => !UseManagedHandler && !PlatformDetection.IsWindows;
+        protected bool IsNetfxHandler => !UseManagedHandler && PlatformDetection.IsWindows && PlatformDetection.IsFullFramework;
+        protected bool IsUapHandler => !UseManagedHandler && PlatformDetection.IsWindows && PlatformDetection.IsUap;
+
         protected HttpClient CreateHttpClient() => new HttpClient(CreateHttpClientHandler());
 
         protected HttpClientHandler CreateHttpClientHandler() => CreateHttpClientHandler(UseManagedHandler);

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

@@ -478,6 +478,33 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>
                 }
             });
         }
+
+        [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]   // Does not support LF-only
+        [Theory]
+        [InlineData("\r\n")]
+        [InlineData("\n")]
+        public async Task GetAsync_ResponseHasNormalLineEndings_Success(string lineEnding)
+        {
+            await LoopbackServer.CreateServerAsync(async (server, url) =>
+            {
+                using (HttpClient client = CreateHttpClient())
+                {
+                    Task<HttpResponseMessage> getResponseTask = client.GetAsync(url);
+                    Task<List<string>> serverTask = LoopbackServer.ReadRequestAndSendResponseAsync(server,
+                        $"HTTP/1.1 200 OK{lineEnding}Date: {DateTimeOffset.UtcNow:R}{lineEnding}Server: TestServer{lineEnding}Content-Length: 0{lineEnding}{lineEnding}",
+                        new LoopbackServer.Options { ResponseStreamWrapper = GetStream });
+
+                    await TestHelper.WhenAllCompletedOrAnyFailed(getResponseTask, serverTask);
+
+                    using (HttpResponseMessage response = await getResponseTask)
+                    {
+                        Assert.Equal(200, (int)response.StatusCode);
+                        Assert.Equal("OK", response.ReasonPhrase);
+                        Assert.Equal("TestServer", response.Headers.Server.ToString());
+                    }
+                }
+            });
+        }
     }
 
     public class HttpProtocolTests_Dribble : HttpProtocolTests