Skip to content

fix(ext/node): emit HPE_INVALID_TRANSFER_ENCODING for CL+chunked respo#33488

Merged
bartlomieju merged 1 commit into
denoland:mainfrom
divybot:claude/test-http-client-reject-chunked-with-content-length
Apr 25, 2026
Merged

fix(ext/node): emit HPE_INVALID_TRANSFER_ENCODING for CL+chunked respo#33488
bartlomieju merged 1 commit into
denoland:mainfrom
divybot:claude/test-http-client-reject-chunked-with-content-length

Conversation

@divybot

@divybot divybot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Enables test-http-client-reject-chunked-with-content-length in node_compat suite.

Test plan

  • cargo test --test node_compat -- test-http-client-reject-chunked-with-content-length

Enables tests/node_compat/runner/suite/test/parallel/test-http-client-reject-chunked-with-content-length.js

@fibibot fibibot 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.

LGTM. The recovery path is well-targeted at the specific request-smuggling case the upstream test exercises:

  • Upstream parallel/test-http-client-reject-chunked-with-content-length.js sends a response with both Content-Length: 1 and Transfer-Encoding: chunked and asserts err.code === 'HPE_INVALID_TRANSFER_ENCODING' plus err.message matching /^Parse Error/.
  • prepareError now checks err.code === "HPE_ERROR" (the generic code the Rust binding returns), runs two ^…[ \t]*: regexes against the latin1-decoded header section of rawPacket, and on a hit overwrites err.code = "HPE_INVALID_TRANSFER_ENCODING" and err.reason. The subsequent err.message = "Parse Error: ${err.reason}" line satisfies the test's regex too.
  • headerSegmentLatin1 correctly bounds the CRLF/CRLF scan with i + 3 < len so the last access (view[i+3]) reads view[len-1]. Falls back to scanning the whole packet when the terminator isn't found, which is fine because the regexes either match or don't.
  • The two regexes (^content-length[ \\t]*: and ^transfer-encoding[ \\t]*:[^\\r\\n]*\\bchunked\\b) use m for multi-line matching and i for case-insensitive — \\bchunked\\b correctly distinguishes from chunked-encoding-style values. Header folding (continuation lines starting with whitespace) isn't handled, but that's deprecated in HTTP/1.1 so unlikely to matter.
  • New primordials (StringFromCharCode, TypedArrayPrototypeGet{Buffer,ByteLength,ByteOffset}) are imported correctly. for (let i = 0; i < end; i++) out += StringFromCharCode(view[i]) is O(n²) on string concat but the header section is small (< a few KB) and this only runs on parse errors.

CI is 0 failures, 18 pending.

@bartlomieju
bartlomieju merged commit 971c90c into denoland:main Apr 25, 2026
112 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