Skip to content

Honor Content-Length when leftover bytes follow HTTP headers - #339

Merged
tylerreckart merged 4 commits into
mainfrom
fix/http-body-honor-content-length
Sep 21, 2026
Merged

tylerreckart merged 4 commits into
mainfrom
fix/http-body-honor-content-length

Conversation

@cursor

@cursor cursor Bot commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

parse_http_request copies the entire header-read leftover into req.body and only recvs more when leftover is shorter than Content-Length. TCP commonly delivers the header sentinel plus extra bytes in one read.

When leftover is longer than CL, those extra bytes became this request's body:

  • Content-Length: 0 with a following payload ran as a real POST body (smuggling / desync).
  • Bytes past a non-zero CL made json_parse fail on trailing characters, or polluted raw bodies.

Outbound ApiClient::read_response_body already takes at most CL bytes from leftover. Inbound did not.

Fix

Cap leftover to want before using it as the body. Short leftover still reads until CL. Connection: close drops the rest (pipelining remains out of scope).

Tests

unit_http_request — exact CL, leftover overshoot, CL:0 does not adopt leftover, GET without CL drops leftover, short body rejected, duplicate CL rejected, Transfer-Encoding rejected. 7/7 locally (+ ASan/UBSan).

Independently mergeable against main (0456350). Code hunks (src/api/http_request.cpp, tests/test_http_request.cpp, CMakeLists.txt unit target) do not overlap #321–#338. git merge-tree --write-tree auto-merges those siblings; CHANGELOG [Unreleased] is the usual sibling note.

Open in Web View Automation 

Note

Medium Risk
Changes request-body boundaries on the API ingress path (api_server uses parse_http_request); incorrect framing could reject valid clients or still mis-handle edge cases, but the change tightens CL semantics rather than loosening them.

Overview
Fixes inbound HTTP/1.1 body framing in parse_http_request: when the first read after headers includes more bytes than Content-Length, the parser now truncates the header leftover to at most want bytes before treating it as the body (then still recvs until the declared length). Previously the entire leftover became the body, so Content-Length: 0 could still pick up a following payload, and overshoot bytes could break JSON parsing or widen smuggling/desync risk. Behavior is aligned with outbound ApiClient::read_response_body.

Adds unit_http_request (tests/test_http_request.cpp) covering exact CL, leftover overshoot, CL:0, GET without CL ignoring pipelined tail, short body failure, duplicate Content-Length, and rejected Transfer-Encoding. Documents the fix under [Unreleased] in CHANGELOG.md.

Reviewed by Cursor Bugbot for commit 8b2639d. Bugbot is set up for automated code reviews on this repo. Configure here.

parse_http_request assigned the entire header-read leftover as the body
and only recv'd more when leftover was shorter than Content-Length. Extra
bytes from the same TCP read (CL:0 with a payload, or bytes past CL) were
adopted as this request's body, so json_parse could fail on trailing junk
or a smuggled POST body could run. Cap leftover to want, matching outbound
read_response_body framing.

Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
Keep both Unreleased changelog entries.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_dc26fd79-f08d-46c3-85f7-9622322eb1f6)

Keep both Unreleased changelog entries.

Co-authored-by: Cursor <cursoragent@cursor.com>
Keep both Unreleased changelog entries.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tylerreckart
tylerreckart merged commit 4656277 into main Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants