Skip to content

Fix consumeChunks: actually strip chunk size-lines and trailing CRLFs#2

Merged
joncol merged 1 commit intomasterfrom
fix-dechunker
May 5, 2026
Merged

Fix consumeChunks: actually strip chunk size-lines and trailing CRLFs#2
joncol merged 1 commit intomasterfrom
fix-dechunker

Conversation

@joncol
Copy link
Copy Markdown

@joncol joncol commented May 5, 2026

Previously consumeChunks returned the raw chunked stream unchanged, so any caller using Transfer-Encoding: chunked input got chunk-size hex prefixes embedded in the body. Multipart parsing in particular saw hex digits inside the file content and rejected the upload.

This fixes consumeChunks/consumeChunksImpl to extract just the chunk body bytes (dropping the size-line and trailing CRLF), and to drop the HTTP trailer from the body output (trailers are headers per RFC 2616, not body content). consumeChunks is also exported from Internal.Handler so it can be tested.

@joncol joncol requested a review from Siprj May 5, 2026 11:43
Copy link
Copy Markdown

@Siprj Siprj left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@joncol joncol requested a review from arybczak May 5, 2026 12:11
@joncol
Copy link
Copy Markdown
Author

joncol commented May 5, 2026

If this is approved, I'll get the latest upstream changes and probably drop the 2 commits that are not committed upstream — this fork hasn't been used in a while.

@arybczak
Copy link
Copy Markdown

arybczak commented May 5, 2026

Maybe put some comments of the PR into the code so it's clearer to look at in the future, otherwise looks alright.

Before merging please force reset the master in this repo to upstream's master and after when you have a moment, make a PR upstream.

Previously consumeChunks returned the raw chunked stream unchanged, so
any caller using Transfer-Encoding: chunked input got chunk-size hex
prefixes embedded in the body. Multipart parsing in particular saw
hex digits inside the file content and rejected the upload.

This fixes consumeChunks/consumeChunksImpl to extract just the chunk
body bytes (dropping the size-line and trailing CRLF), and to drop
the HTTP trailer from the body output (trailers are headers per
RFC 2616, not body content). consumeChunks is also exported from
Internal.Handler so it can be tested.
@joncol
Copy link
Copy Markdown
Author

joncol commented May 5, 2026

@arybczak, I assume I don't need to care about the GHC 9.14 errors on CI?

@joncol
Copy link
Copy Markdown
Author

joncol commented May 5, 2026

Maybe put some comments of the PR into the code so it's clearer to look at in the future, otherwise looks alright.

I added some comments to the code.

Before merging please force reset the master in this repo to upstream's master and after when you have a moment, make a PR upstream.

Fork has been re-set, and PR rebased on top of latest master.

I've created an upstream PR here: Happstack#91

@joncol joncol merged commit 7978f85 into master May 5, 2026
13 of 14 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.

3 participants