diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs index 5b346164b93b..a8c2916c0247 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs @@ -4,7 +4,9 @@ using System.Diagnostics; using System.IO.Pipelines; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; @@ -159,6 +161,26 @@ public static MessageBody For( KestrelBadHttpRequestException.Throw(RequestRejectionReason.FinalTransferCodingNotChunked, transferEncoding); } + // 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 (headers.ContentLength.HasValue) + { + IHeaderDictionary headerDictionary = headers; + headerDictionary.Add("X-Content-Length", headerDictionary[HeaderNames.ContentLength]); + headers.ContentLength = null; + } + // TODO may push more into the wrapper rather than just calling into the message body // NBD for now. return new Http1ChunkedEncodingMessageBody(context, keepAlive); diff --git a/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs b/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs index 45092dd97e8a..8091afec1044 100644 --- a/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs +++ b/src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Primitives; +using Microsoft.Net.Http.Headers; using Moq; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests; @@ -1014,6 +1015,25 @@ public void BadRequestFor11BadHostHeaderFormat() Assert.Equal(CoreStrings.FormatBadRequest_InvalidHostHeader_Detail("a=b"), ex.Message); } + [Fact] + public void ContentLengthShouldBeRemovedWhenBothTransferEncodingAndContentLengthRequestHeadersExist() + { + // Arrange + string contentLength = "1024"; + _http1Connection.RequestHeaders.Add(HeaderNames.ContentLength, contentLength); + _http1Connection.RequestHeaders.Add(HeaderNames.TransferEncoding, "chunked"); + + // Act + Http1MessageBody.For(Kestrel.Core.Internal.Http.HttpVersion.Http11, (HttpRequestHeaders)_http1Connection.RequestHeaders, _http1Connection); + + // Assert + Assert.True(_http1Connection.RequestHeaders.ContainsKey("X-Content-Length")); + Assert.Equal(contentLength, _http1Connection.RequestHeaders["X-Content-Length"]); + Assert.True(_http1Connection.RequestHeaders.ContainsKey(HeaderNames.TransferEncoding)); + Assert.Equal("chunked", _http1Connection.RequestHeaders[HeaderNames.TransferEncoding]); + Assert.False(_http1Connection.RequestHeaders.ContainsKey(HeaderNames.ContentLength)); + } + private bool TakeMessageHeaders(ReadOnlySequence readableBuffer, bool trailers, out SequencePosition consumed, out SequencePosition examined) { var reader = new SequenceReader(readableBuffer);