Skip to content

Reject bare CR in managed HttpListener request parsing - #133665

Open
Dev-next-gen wants to merge 1 commit into
dotnet:mainfrom
Dev-next-gen:httplistener-bare-cr
Open

Reject bare CR in managed HttpListener request parsing#133665
Dev-next-gen wants to merge 1 commit into
dotnet:mainfrom
Dev-next-gen:httplistener-bare-cr

Conversation

@Dev-next-gen

Copy link
Copy Markdown

While reading the chunk-size change in #132747 I noticed that the managed HttpListener silently drops a CR that isn't followed by a LF, and glues together the text on either side of it. I replayed a few raw requests against the managed listener (Linux, .NET 8.0.30) and against http.sys (Windows 11, .NET 8.0.29):

request contains managed HttpListener http.sys
Content-Le\rngth: 5 + body Hello context with Content-Length: 5, body read 400
X-Test: ab\rcd header value abcd 400
GET /c\rd HTTP/1.1 RawUrl is /cd 400
Host: localhost\r\r\n accepted 400
chunk-size line 1\r0\n read as a 0x10 byte chunk 400

Both line readers do the same thing. HttpConnection.ReadLine moves to LineState.CR on a CR but keeps appending the bytes that follow, and the next LF ends the line, so a CR anywhere in the request line or a header just disappears. ChunkStream.GetChunkSize behaves the same way. It does have a Missing \n check meant for this case, but it sits after the read loop, and the loop only exits without a break once offset == size, so offset < size is never true there.

RFC 9112 section 2.2 says a recipient of a bare CR must treat the element as invalid or replace the CR with SP. Dropping it does neither, and Content-Le\rngth becoming Content-Length is the same kind of disagreement with an intermediary that #132163 and #132747 closed for whitespace.

The change rejects a CR that isn't immediately followed by a LF in both places: ReadLine throws, which ProcessInput already turns into a 400, and the Missing \n check in GetChunkSize moves into the loop where it can fire. A bare LF is still accepted as a line terminator, and a CR and LF split across two reads still work, since the CR state carries over between calls as before.

On the behavior change: the public API is untouched, and the only requests that now fail are ones that are invalid per the RFC and that http.sys already rejects with a 400 behind the same API, so this brings the managed implementation in line with Windows, as #130910 did for Content-Length.

Tests:

  • four bare-CR entries in InvalidClientRequestTests, not gated on the implementation. I replayed exactly those four requests, built the way GetContext_InvalidRequest_DoesNotGetContext builds them, against http.sys on Windows 11 and each got 400 Bad Request; against the shipped managed listener each one got a context.
  • Read_ChunkSizeWithBareCR_ThrowsHttpListenerException, managed only, next to the whitespace tests, because http.sys answers 400 before a context is handed out.

I couldn't build the repo on the machines I had (no .NET 11 SDK), so I have not run the xunit tests themselves and am relying on CI for those. What I did run: a net8.0 harness that compiles ChunkStream.cs from the tree as-is and HttpConnection.ReadLine extracted verbatim. At main, 7 of its bare-CR checks fail; with this change they all pass, and the healthy-path checks (CR/LF split across reads, byte-by-byte input, bare LF endings, chunk extensions, several chunks, whitespace in the chunk size still rejected) pass on both sides.

I also noticed that ChunkStream.ReadTrailer never appends the trailer characters themselves to _saved, so trailer fields are dropped, and that 0\r\n\r\r\n leaves the stream waiting for more data. I left that alone here; I can open an issue if it's useful.

AI tools used

The managed HttpListener dropped a CR that wasn't followed by a LF and
joined the text on either side of it, both in the request line and
headers (HttpConnection.ReadLine) and in chunk-size lines
(ChunkStream.GetChunkSize). "Content-Le\rngth: 5" was treated as a
Content-Length header and "1\r0\n" as a 0x10 byte chunk, while http.sys
rejects all of these with a 400.

Reject the bare CR instead. In ChunkStream this moves the existing
"Missing \n" check to where it can actually fire; it sat after the read
loop, where offset < size is always false.
@azure-pipelines

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

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Sep 11, 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.

@Dev-next-gen

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

@Dev-next-gen

Copy link
Copy Markdown
Author

@Dev-next-gen please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@azure-pipelines

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

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

Labels

area-System.Net.Http community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant