fix dotnet#43523 HttpSys Content-Length and Transfer-Encoding conflicts

src/Servers/HttpSys/src/RequestProcessing/Request.cs

@@ -8,8 +8,10 @@
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
 using System.Security.Principal;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.HttpSys.Internal;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
 
 namespace Microsoft.AspNetCore.Server.HttpSys;
@@ -108,6 +110,8 @@ internal Request(RequestContext requestContext)
         // Finished directly accessing the HTTP_REQUEST structure.
         RequestContext.ReleasePins();
         // TODO: Verbose log parameters
+
+        RemoveContentLengthIfTransferEncodingContainsChunked();
     }
 
     internal ulong UConnectionId { get; }
@@ -116,7 +120,7 @@ internal Request(RequestContext requestContext)
 
     // No ulongs in public APIs...
     public long ConnectionId => RawConnectionId != 0 ? (long)RawConnectionId : (long)UConnectionId;
-    
+
     internal ulong RequestId { get; }
 
     private SslStatus SslStatus { get; }
@@ -134,7 +138,7 @@ public long? ContentLength
             {
                 // Note Http.Sys adds the Transfer-Encoding: chunked header to HTTP/2 requests with bodies for back compat.
                 var transferEncoding = Headers[HeaderNames.TransferEncoding].ToString();
-                if (string.Equals("chunked", transferEncoding.Trim(), StringComparison.OrdinalIgnoreCase))
+                if (transferEncoding != null && string.Equals("chunked", transferEncoding.Split(',')[^1].Trim(), StringComparison.OrdinalIgnoreCase))
                 {
                     _contentBoundaryType = BoundaryType.Chunked;
                 }
@@ -458,4 +462,32 @@ private static partial class Log
         [LoggerMessage(LoggerEventIds.ErrorInReadingCertificate, LogLevel.Debug, "An error occurred reading the client certificate.", EventName = "ErrorInReadingCertificate")]
         public static partial void ErrorInReadingCertificate(ILogger logger, Exception exception);
     }
+
+    private void RemoveContentLengthIfTransferEncodingContainsChunked()
+    {
+        if (StringValues.IsNullOrEmpty(Headers.ContentLength)) { return; }
+
+        var transferEncoding = Headers[HeaderNames.TransferEncoding].ToString();
+        if (transferEncoding == null || !string.Equals("chunked", transferEncoding.Split(',')[^1].Trim(), StringComparison.OrdinalIgnoreCase))
+        {
+            return;
+        }
+
+        // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2
+        // A sender MUST NOT send a Content-Length header field in any message
+        // that contains a Transfer-Encoding header field.
+        // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3
+        // If a message is received with both a Transfer-Encoding and a
+        // Content-Length header field, the Transfer-Encoding overrides the
+        // Content-Length.  Such a message might indicate an attempt to
+        // perform request smuggling (Section 9.5) or response splitting
+        // (Section 9.4) and ought to be handled as an error.  A sender MUST
+        // remove the received Content-Length field prior to forwarding such
+        // a message downstream.
+        // We should remove the Content-Length request header in this case, for compatibility
+        // reasons, include X-Content-Length so that the original Content-Length is still available.
+        IHeaderDictionary headerDictionary = Headers;
+        headerDictionary.Add("X-Content-Length", headerDictionary[HeaderNames.ContentLength]);
+        Headers.ContentLength = StringValues.Empty;
+    }
 }

src/Servers/HttpSys/test/FunctionalTests/RequestHeaderTests.cs

@@ -1,15 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
-using System.Collections.Generic;
 using System.Net.Http;
 using System.Net.Sockets;
 using System.Text;
-using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.Primitives;
-using Xunit;
 
 namespace Microsoft.AspNetCore.Server.HttpSys;
 
@@ -78,6 +75,84 @@ public async Task RequestHeaders_ServerAddsCustomHeaders_Success()
         }
     }
 
+    [ConditionalFact]
+    public async Task RequestHeaders_ClientSendTransferEncodingHeaders()
+    {
+        string address;
+        using (Utilities.CreateHttpServer(out address, httpContext =>
+        {
+            var requestHeaders = httpContext.Request.Headers;
+            var request = httpContext.Features.Get<RequestContext>().Request;
+            Assert.Single(requestHeaders["Transfer-Encoding"]);
+            Assert.Equal("chunked", requestHeaders.TransferEncoding);
+            Assert.True(request.HasEntityBody);
+            return Task.FromResult(0);
+        }))
+        {
+            var headerDictionary = new HeaderDictionary(new Dictionary<string, StringValues> {
+                { "Transfer-Encoding", "chunked" }
+            });
+            var response = await SendRequestAsync(address, headerDictionary);
+            var responseStatusCode = response.Substring(9, 3); // Skip "HTTP/1.1 "
+            Assert.Equal("200", responseStatusCode);
+        }
+    }
+
+    [ConditionalFact]
+    public async Task RequestHeaders_ClientSendTransferEncodingHeadersWithMultipleValues()
+    {
+        string address;
+        using (Utilities.CreateHttpServer(out address, httpContext =>
+        {
+            var requestHeaders = httpContext.Request.Headers;
+            var request = httpContext.Features.Get<RequestContext>().Request;
+            Assert.Single(requestHeaders["Transfer-Encoding"]);
+            Assert.Equal("gzip, chunked", requestHeaders.TransferEncoding);
+            Assert.True(request.HasEntityBody);
+            return Task.FromResult(0);
+        }))
+        {
+            var headerDictionary = new HeaderDictionary(new Dictionary<string, StringValues> {
+                { "Transfer-Encoding", new string[] { "gzip", "chunked" } }
+            });
+            var response = await SendRequestAsync(address, headerDictionary);
+            var responseStatusCode = response.Substring(9, 3); // Skip "HTTP/1.1 "
+            Assert.Equal("200", responseStatusCode);
+        }
+    }
+
+    [ConditionalFact]
+    public async Task RequestHeaders_ClientSendTransferEncodingAndContentLength_ContentLengthShouldBeRemoved()
+    {
+        string address;
+        using (Utilities.CreateHttpServer(out address, httpContext =>
+        {
+            var requestHeaders = httpContext.Request.Headers;
+            var request = httpContext.Features.Get<RequestContext>().Request;
+            Assert.Single(requestHeaders["Transfer-Encoding"]);
+            Assert.Equal("gzip, chunked", requestHeaders.TransferEncoding);
+
+            Assert.Null(request.ContentLength);
+            Assert.True(request.HasEntityBody);
+
+            Assert.False(requestHeaders.ContainsKey("Content-Length"));
+            Assert.Null(requestHeaders.ContentLength);
+
+            Assert.Single(requestHeaders["X-Content-Length"]);
+            Assert.Equal("1", requestHeaders["X-Content-Length"]);
+            return Task.FromResult(0);
+        }))
+        {
+            var headerDictionary = new HeaderDictionary(new Dictionary<string, StringValues> {
+                { "Transfer-Encoding", new string[] { "gzip", "chunked" } },
+                { "Content-Length", "1" },
+            });
+            var response = await SendRequestAsync(address, headerDictionary);
+            var responseStatusCode = response.Substring(9, 3); // Skip "HTTP/1.1 "
+            Assert.Equal("200", responseStatusCode);
+        }
+    }
+
     private async Task<string> SendRequestAsync(string uri)
     {
         using (HttpClient client = new HttpClient())
@@ -114,4 +189,35 @@ private async Task SendRequestAsync(string address, string customHeader, string[
         await Task.Run(() => socket.Receive(response));
         socket.Dispose();
     }
+
+    private async Task<string> SendRequestAsync(string address, IHeaderDictionary headers)
+    {
+        var uri = new Uri(address);
+        StringBuilder builder = new StringBuilder();
+        builder.AppendLine("POST / HTTP/1.1");
+        builder.AppendLine("Connection: close");
+        builder.Append("HOST: ");
+        builder.AppendLine(uri.Authority);
+        foreach (var header in headers)
+        {
+            foreach (var value in header.Value)
+            {
+                builder.Append(header.Key);
+                builder.Append(": ");
+                builder.AppendLine(value);
+            }
+        }
+        builder.AppendLine();
+
+        byte[] request = Encoding.ASCII.GetBytes(builder.ToString());
+
+        using (Socket socket = new Socket(SocketType.Stream, ProtocolType.Tcp))
+        {
+            socket.Connect(uri.Host, uri.Port);
+            socket.Send(request);
+            byte[] response = new byte[1024 * 5];
+            var read = await Task.Run(() => socket.Receive(response));
+            return Encoding.ASCII.GetString(response, 0, read);
+        }
+    }
 }

src/Servers/IIS/IIS/src/Core/IISHttpContext.cs

@@ -259,8 +259,26 @@ private bool CheckRequestCanHaveBody()
         // Note Http.Sys adds the Transfer-Encoding: chunked header to HTTP/2 requests with bodies for back compat.
         // Transfer-Encoding takes priority over Content-Length.
         string transferEncoding = RequestHeaders.TransferEncoding.ToString();
-        if (string.Equals("chunked", transferEncoding?.Trim(), StringComparison.OrdinalIgnoreCase))
-        {
+        if (transferEncoding != null && string.Equals("chunked", transferEncoding.Split(',')[^1].Trim(), StringComparison.OrdinalIgnoreCase))
+        {
+            // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2
+            // A sender MUST NOT send a Content-Length header field in any message
+            // that contains a Transfer-Encoding header field.
+            // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3
+            // If a message is received with both a Transfer-Encoding and a
+            // Content-Length header field, the Transfer-Encoding overrides the
+            // Content-Length.  Such a message might indicate an attempt to
+            // perform request smuggling (Section 9.5) or response splitting
+            // (Section 9.4) and ought to be handled as an error.  A sender MUST
+            // remove the received Content-Length field prior to forwarding such
+            // a message downstream.
+            // We should remove the Content-Length request header in this case, for compatibility
+            // reasons, include X-Content-Length so that the original Content-Length is still available.
+            if (RequestHeaders.ContentLength.HasValue)
+            {
+                RequestHeaders.Add("X-Content-Length", RequestHeaders[HeaderNames.ContentLength]);
+                RequestHeaders.ContentLength = null;
+            }
             return true;
         }
 

src/Servers/IIS/IIS/test/Common.FunctionalTests/RequestResponseTests.cs

@@ -718,6 +718,47 @@ await connection.Send(
         await Task.WhenAll(tasks);
     }
 
+    [ConditionalFact]
+    [RequiresNewHandler]
+    public async Task SendTransferEncodingHeadersWithMultipleValues()
+    {
+        using (var connection = _fixture.CreateTestConnection())
+        {
+            await connection.Send(
+                "POST /TransferEncodingHeadersWithMultipleValues HTTP/1.1",
+                "Transfer-Encoding: gzip, chunked",
+                "Host: localhost",
+                "Connection: close",
+                "",
+                "");
+
+            await connection.Receive(
+                "HTTP/1.1 200 OK",
+                "");
+        }
+    }
+
+    [ConditionalFact]
+    [RequiresNewHandler]
+    public async Task SendTransferEncodingAndContentLength_ContentLengthShouldBeRemoved()
+    {
+        using (var connection = _fixture.CreateTestConnection())
+        {
+            await connection.Send(
+                "POST /TransferEncodingAndContentLengthShouldBeRemove HTTP/1.1",
+                "Transfer-Encoding: gzip, chunked",
+                "Content-Length: 5",
+                "Host: localhost",
+                "Connection: close",
+                "",
+                "");
+
+            await connection.Receive(
+                "HTTP/1.1 200 OK",
+                "");
+        }
+    }
+
     private async Task<(int Status, string Body)> SendSocketRequestAsync(string path)
     {
         using (var connection = _fixture.CreateTestConnection())

src/Servers/IIS/IIS/test/testassets/InProcessWebSite/Startup.cs

@@ -1061,6 +1061,45 @@ public Task InvalidCharacter(HttpContext context)
         return Task.CompletedTask;
     }
 
+    private async Task TransferEncodingHeadersWithMultipleValues(HttpContext ctx)
+    {
+        try
+        {
+#if !FORWARDCOMPAT
+        Assert.True(ctx.Request.CanHaveBody());
+#endif
+            Assert.True(ctx.Request.Headers.ContainsKey("Transfer-Encoding"));
+            Assert.Equal("gzip, chunked", ctx.Request.Headers["Transfer-Encoding"]);
+            return;
+        }
+        catch (Exception exception)
+        {
+            ctx.Response.StatusCode = 500;
+            await ctx.Response.WriteAsync(exception.ToString());
+        }
+    }
+
+    private async Task TransferEncodingAndContentLengthShouldBeRemove(HttpContext ctx)
+    {
+        try
+        {
+#if !FORWARDCOMPAT
+        Assert.True(ctx.Request.CanHaveBody());
+#endif
+            Assert.True(ctx.Request.Headers.ContainsKey("Transfer-Encoding"));
+            Assert.Equal("gzip, chunked", ctx.Request.Headers["Transfer-Encoding"]);
+            Assert.False(ctx.Request.Headers.ContainsKey("Content-Length"));
+            Assert.True(ctx.Request.Headers.ContainsKey("X-Content-Length"));
+            Assert.Equal("5", ctx.Request.Headers["X-Content-Length"]);
+            return;
+        }
+        catch (Exception exception)
+        {
+            ctx.Response.StatusCode = 500;
+            await ctx.Response.WriteAsync(exception.ToString());
+        }
+    }
+
 #if !FORWARDCOMPAT
     public Task ResponseTrailers_HTTP2_TrailersAvailable(HttpContext context)
     {