Kestrel doesn't reject some invalid Transfer-Encoding headers

[Tratcher]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

[Offline report]

chunked is supposed to be the last value in the Transfer-Encoding header and servers are supposed to reject requests that have another value at the end.

https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.3

3. If a Transfer-Encoding header field is present and the chunked
   transfer coding (Section 4.1) is the final encoding, the message
   body length is determined by reading and decoding the chunked
   data until the transfer coding indicates the data is complete.

   If a Transfer-Encoding header field is present in a response and
   the chunked transfer coding is not the final encoding, the
   message body length is determined by reading the connection until
   it is closed by the server. If a Transfer-Encoding header field
   is present in a request and the chunked transfer coding is not
   the final encoding, the message body length cannot be determined
   reliably; the server MUST respond with the 400 (Bad Request)
   status code and then close the connection.

https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.1
If any transfer coding
other than chunked is applied to a request payload body, the sender
MUST apply chunked as the final transfer coding to ensure that the
message is properly framed.

A server that receives a request message with a transfer coding it
does not understand SHOULD respond with 501 (Not Implemented).

Takeaways:
· Transfer-Encoding is an order sensitive multi-value header. In practice the only value used is 'chunked'.
· If there is an unknown value before 'chunked' the server/app should respond with a 501. We currently delegate that responsibility to middleware.
· If 'chunked' is not last, the server should respond with a 400. This is a bug with Kestrel's current implementation.
Kestrel does have this check that should produce the 400:

aspnetcore/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs

Lines 149 to 160 in 94cc679

	var transferCoding = HttpHeaders.GetFinalTransferCoding(transferEncoding);
	// https://tools.ietf.org/html/rfc7230#section-3.3.3
	// If a Transfer-Encoding header field
	// is present in a request and the chunked transfer coding is not
	// the final encoding, the message body length cannot be determined
	// reliably; the server MUST respond with the 400 (Bad Request)
	// status code and then close the connection.
	if (transferCoding != TransferCoding.Chunked)
	{
	KestrelBadHttpRequestException.Throw(RequestRejectionReason.FinalTransferCodingNotChunked, transferEncoding);
	}

However, there are some bugs where GetFinalTransferCoding doesn't work correctly depending on the input. 'z' is one case and 'chunk' is another. These should be fixed & patched.

Versions affected: AspNetCore 6, 7, 8
Not affected: AspNetCore 2.1, 3.1

This was probably regressed by refactoring this method in 5.0: https://github.com/dotnet/aspnetcore/pull/21202/files

Expected Behavior

Invalid Transfer-Encoding headers should cause the request to be rejected with a 400.

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

[BrennanConroy]

I think you meant likely regressed by https://github.com/dotnet/aspnetcore/pull/21202/files