Fix token-list parsing for hop-by-hop request headers#39
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f9b909f95f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| seenTransferEncoding = true; | ||
| isChunked = Ascii.EqualsIgnoreCase ( valueSpan, ChunkedValue ); | ||
| isChunked = TokenListContains ( valueSpan, ChunkedValue ); |
There was a problem hiding this comment.
Reject non-final chunked transfer coding
TokenListContains treats any occurrence of chunked as sufficient, so a request like Transfer-Encoding: chunked, gzip will be accepted as chunked (isChunked = true) instead of rejected. In HTTP/1.1 this is invalid framing (chunked must be the final transfer-coding), and accepting it can create request-boundary mismatches with intermediaries/backends that follow RFC 9112 and return 400, which is a request-smuggling risk on reused connections.
Useful? React with 👍 / 👎.
|
@codex address those comments |
Summary
Testing
Git / PR actions performed
|
Motivation
Transfer-Encoding: gzip, chunkedto be mis-parsed and leavingContent-Lengthtrusted.Description
TokenListContains(ReadOnlySpan<byte> value, ReadOnlySpan<byte> expectedToken)that scans comma-separated tokens with trimming and case-insensitive comparison.Connection,Expect, andTransfer-Encodingheaders insideHttpRequestReadersoConnection: close,Expect: 100-continue, andTransfer-Encoding: ..., chunkedare recognized correctly.ContentLength(contentLength = -1) when achunkedtransfer-coding is observed and kept the duplicate-TE and Host validations intact inHttpRequestReader.Testing
dotnet testinitially against an incorrect project path which failed because the specified project file did not exist.dotnet test tests/Sisk.Core/tests.csproj --filter CadenteEngineRareTests --nologo, and the project restore/build completed successfully..NETCore.App 9.0.0which is not installed in this environment, so tests could not be executed here (build succeeded, but test runtime was unavailable).Codex Task