Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.IO.Pipelines;
using Microsoft.AspNetCore.Connections;
using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
using Microsoft.Net.Http.Headers;

namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http;

Expand Down Expand Up @@ -159,6 +160,25 @@ 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, increase x-Content-Length so that the original Content-Length is still available.
Comment thread
Tratcher marked this conversation as resolved.
Outdated
if (headers.ContentLength.HasValue)
Comment thread
FatTigerWang marked this conversation as resolved.
{
context.RequestHeaders.Add($"x-{HeaderNames.ContentLength}", context.RequestHeaders[HeaderNames.ContentLength]);
context.RequestHeaders.Remove(HeaderNames.ContentLength);
Comment thread
FatTigerWang marked this conversation as resolved.
Outdated
}

// TODO may push more into the wrapper rather than just calling into the message body
// NBD for now.
return new Http1ChunkedEncodingMessageBody(context, keepAlive);
Expand Down
18 changes: 18 additions & 0 deletions src/Servers/Kestrel/Core/test/Http1/Http1ConnectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1014,6 +1015,23 @@ 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-{HeaderNames.ContentLength}"));
Comment thread
Tratcher marked this conversation as resolved.
Outdated
Assert.Equal(contentLength, _http1Connection.RequestHeaders[$"x-{HeaderNames.ContentLength}"]);
Assert.False(_http1Connection.RequestHeaders.ContainsKey(HeaderNames.ContentLength));
}

private bool TakeMessageHeaders(ReadOnlySequence<byte> readableBuffer, bool trailers, out SequencePosition consumed, out SequencePosition examined)
{
var reader = new SequenceReader<byte>(readableBuffer);
Expand Down