Skip to content

Decode chunked request bodies; reject invalid framing per RFC 9112#210

Merged
swhitty merged 2 commits into
swhitty:mainfrom
ianegordon:ian/tvt-287-chunked-request-bodies
Apr 26, 2026
Merged

Decode chunked request bodies; reject invalid framing per RFC 9112#210
swhitty merged 2 commits into
swhitty:mainfrom
ianegordon:ian/tvt-287-chunked-request-bodies

Conversation

@ianegordon

Copy link
Copy Markdown
Contributor

Summary

  • HTTPDecoder.readBody now honors Transfer-Encoding: chunked (RFC 9112 §7.1) by routing the body through a new HTTPChunkedTransferDecoder — the read-side mirror of the existing HTTPChunkedTransferEncoder. Trailer fields (RFC 9112 §7.1.2) are consumed and discarded.
  • readBody also throws HTTPDecoder.Error on framing violations:
    • both Content-Length and Transfer-Encoding present (§6.1, smuggling prevention)
    • non-numeric or negative Content-Length (§6.3 Added most of the HTTP status codes and phrases #5, "unrecoverable error")
    • Transfer-Encoding whose final coding is not chunked (§6.1)
  • Throwing surfaces as a connection-close, which is RFC-permitted for unrecoverable framing errors. HTTPServer.handleConnection has a // TODO to upgrade this to an explicit 400 Bad Request response in a future change.
  • readBody's signature changes from (from:length:) to (from:contentLength:transferEncoding:). Both decodeRequest and decodeResponse now pass the relevant headers.

Closes TVT-287.

Test plan

  • Six new HTTPDecoderTests cases covering chunked multi-chunk decode, terminator-only, trailer ignore, conflicting CL+TE, non-numeric CL, negative CL.
  • Existing readBody/test-helper call sites updated to the new signature.
  • Full suite: 414 tests, 49 suites, all passing locally.

HTTPDecoder.readBody now honors Transfer-Encoding: chunked (RFC 9112
§7.1) by routing the body through a new HTTPChunkedTransferDecoder, the
read-side mirror of HTTPChunkedTransferEncoder. Trailer fields are
consumed and discarded.

readBody also throws HTTPDecoder.Error on framing violations:
- both Content-Length and Transfer-Encoding present (§6.1)
- non-numeric or negative Content-Length (§6.3 swhitty#5)
- Transfer-Encoding whose final coding is not `chunked` (§6.1)

Throwing surfaces as a connection-close (RFC-permitted for unrecoverable
framing errors); HTTPServer.handleConnection has a TODO to upgrade this
to an explicit 400 Bad Request response in a future change.

readBody's signature changes from (from:length:) to
(from:contentLength:transferEncoding:). Both decodeRequest and
decodeResponse pass headers[.contentLength] and headers[.transferEncoding].

Closes TVT-287

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@codecov

codecov Bot commented Apr 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.60440% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.74%. Comparing base (4a8c239) to head (5326698).

Files with missing lines Patch % Lines
FlyingFox/Sources/HTTPChunkedDecodedSequence.swift 91.66% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #210      +/-   ##
==========================================
+ Coverage   92.67%   92.74%   +0.06%     
==========================================
  Files          69       70       +1     
  Lines        3551     3638      +87     
==========================================
+ Hits         3291     3374      +83     
- Misses        260      264       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Six additional HTTPDecoderTests targeting the uncovered branches in
HTTPChunkedTransferDecoder and HTTPDecoder.readBody:
- chunkExt_IsIgnored (RFC 9112 §7.1: chunk-ext after `;` is parsed but ignored)
- invalidChunkSize_ThrowsError (non-hex chunk-size)
- missingCRLFAfterChunkData_ThrowsError (chunk-data not followed by CRLF)
- truncatedChunkSize_ThrowsError / truncatedChunkData_ThrowsError
  (stream ends mid-line / mid-chunk -> SocketError.disconnected)
- unsupportedTransferEncoding_ThrowsError (e.g. `gzip`)

Lifts HTTPChunkedDecodedSequence.swift to 86.79%/88.73% region/line
coverage and HTTPDecoder.swift's TVT-287 additions to 100%.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@swhitty swhitty merged commit 5295843 into swhitty:main Apr 26, 2026
13 checks passed
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.

2 participants