Skip to content

Strictly require CRLF when parsing HTTP/1.1 chunked encoding lines#131138

Open
MihaZupan with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-http-chunked-encoding
Open

Strictly require CRLF when parsing HTTP/1.1 chunked encoding lines#131138
MihaZupan with Copilot wants to merge 4 commits into
mainfrom
copilot/fix-http-chunked-encoding

Conversation

Copilot AI commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Per RFC 9112, interpreting a lone LF as CRLF is only permitted for the status line and headers — chunked encoding data lines must be terminated by CRLF. SocketsHttpHandler used the same lenient line parsing everywhere, so it accepted lone CR bytes in chunk extensions and a single LF for chunk termination.

Changes

  • HttpConnection.TryReadNextChunkedLine: Reworked to strictly require CRLF for the chunk size line and chunk terminator. It now locates the first CR/LF and rejects a bare LF (lone-LF termination) or a CR not immediately followed by LF (bare CR in size line/extension). The MaxChunkBytesAllowed limit and partial-buffer "wait for more data" behavior are unchanged.
  • Strings.resx: Added net_http_invalid_response_chunk_line_ending, thrown as HttpIOException(InvalidResponse, ...).
  • Tests: New SocketsHttpHandler_ChunkedEncodingParsing_Test covering valid CRLF (incl. chunk extensions and trailers) and rejection of lone-LF at multiple positions and bare CR in the size line/extension.

Scope note

Chunked trailers (after 0\r\n) are parsed by the header parser and remain intentionally lenient per spec; this change only tightens the chunk size line and chunk terminator.

Behavioral delta (all valid CRLF inputs unchanged):

Input Before After
4\r\n accept accept
4\n accept reject
4;ext\rval\r\n (bare CR in ext) accept reject
data\n terminator (lone LF) accept reject

Copilot AI review requested due to automatic review settings July 21, 2026 10:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
16 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@github-actions github-actions Bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 21, 2026
Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 21, 2026 11:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 21, 2026 11:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI changed the title [WIP] Fix parsing of HTTP/1.1 chunked encoding lines Strictly require CRLF when parsing HTTP/1.1 chunked encoding lines Jul 21, 2026
@MihaZupan
MihaZupan requested a review from Copilot July 21, 2026 11:09
Copilot AI requested a review from MihaZupan July 21, 2026 11:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs Outdated
@github-actions github-actions Bot added area-System.Net.Http and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 21, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @karelz, @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@MihaZupan MihaZupan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Address all review feedback

Comment thread src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs Outdated
Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 21, 2026 11:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because there is no eligible user to bill. To allow Copilot reviews on bot-authored pull requests, enable direct organization billing in your organization's Copilot settings.

Copilot AI commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Address all review feedback

Addressed in d3a2545.

Updated the HttpConnection bounds checks to the unsigned idiom, reformatted the loopback response send into a separate block, and tightened the chunked parsing test to assert HttpRequestError.InvalidResponse / HttpIOException while using IgnoreExceptions(...) for the server-side race.

Note

This comment was created by GitHub Copilot.

Copilot AI requested a review from MihaZupan July 21, 2026 12:10
@MihaZupan MihaZupan added this to the 11.0.0 milestone Jul 21, 2026
@MihaZupan
MihaZupan marked this pull request as ready for review July 21, 2026 14:24
Copilot AI review requested due to automatic review settings July 21, 2026 14:24
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 4 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

More strictly follow the HTTP spec when parsing HTTP/1.1 chunked encoding lines

3 participants