Skip to content

fix: reject cross-realm FormData request bodies#5521

Open
Pyker wants to merge 2 commits into
nodejs:mainfrom
Pyker:fix/cross-realm-formdata-request-body
Open

fix: reject cross-realm FormData request bodies#5521
Pyker wants to merge 2 commits into
nodejs:mainfrom
Pyker:fix/cross-realm-formdata-request-body

Conversation

@Pyker

@Pyker Pyker commented Jul 7, 2026

Copy link
Copy Markdown

This relates to...

Fixes #5520. Related context: #5500 / #5502 (legacy dispatcher bridge), #5016 and #4285 (fetch-side cross-realm FormData behavior, unchanged here).

Rationale

request() accepts FormData bodies through the duck-typed util.isFormDataLike() check, but only undici's own FormData can be encoded: the multipart extraction is gated on the webidl.is.FormData brand check. A FormData from another realm (Node.js' builtin globalThis.FormData, another undici copy, a polyfill) passed the gate, fell through the extraction, and was dispatched as a body that never produced any bytes: the request hung silently until an external timeout, with zero bytes written to the socket (full version matrix and reproductions in #5520).

Per review direction (encode vs reject, maintainers chose reject): such bodies now fail fast with InvalidArgumentError. The check lives in the Request constructor, where other invalid body types are already rejected, so both the HTTP/1 and HTTP/2 clients reject cleanly at dispatch time and the fetch code is untouched. The FormData brand check is loaded lazily so the fetch machinery is not pulled in upfront.

Note for changelog purposes: this is a behavior change relative to v6, which encoded cross-realm FormData successfully (v7.0.0 through 7.19.x failed these bodies with an unrelated TypeError; 7.20.0+ hung).

Changes

  • lib/core/request.js: FormData-like bodies that are not instances of undici's FormData now throw InvalidArgumentError instead of falling through to iterable handling.
  • test/issue-5520.js: cross-realm FormData rejection over HTTP/1 and HTTP/2 (own-FormData encoding is already covered by test/client-request.js and test/http2-body.js).

Features

N/A

Bug Fixes

  • request() with a FormData from another realm now rejects immediately with a clear error instead of hanging forever.

Breaking Changes and Deprecations

N/A (the affected input previously hung or produced a broken body; it never worked on v7/v8)

Status

  • I have read and agreed to the Developer's Certificate of Origin
  • Tested (test/issue-5520.js plus unit, fetch, client-request, http2-body and h2 core suites; lint clean)
  • Benchmarked (optional)
  • Documented (behavior fix; error message self-describes)
  • Review ready
  • In review
  • Merge ready

@KhafraDev KhafraDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This shouldn't be under web/fetch, if this is something we want to support at all.

@Pyker

Pyker commented Jul 7, 2026

Copy link
Copy Markdown
Author

Moved it out of web/fetch in 5faa679: body.js is now untouched and the rebuild lives in lib/dispatcher/formdata-body.js, next to its only consumers.

On supporting it at all: the h1/h2 clients already accept any FormData-like through util.isFormDataLike(), and today that acceptance ends in a silent hang with zero bytes on the wire. So the real choice is encode it (this PR, matches v6) or tighten the gate to the brand check and throw InvalidArgumentError (loud, but breaks request() + Node's global FormData, which worked on v6). I prefer the first; happy to rework to the second if that is the direction you want.

@Pyker Pyker requested a review from KhafraDev July 7, 2026 14:23

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.44%. Comparing base (0dee09d) to head (5faa679).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5521      +/-   ##
==========================================
- Coverage   93.44%   93.44%   -0.01%     
==========================================
  Files         110      111       +1     
  Lines       37329    37367      +38     
==========================================
+ Hits        34883    34918      +35     
- Misses       2446     2449       +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KhafraDev

Copy link
Copy Markdown
Member

Is this something we want to support? We don't support this pattern anywhere else. Wouldn't it make more sense to just throw an error?

@mcollina

mcollina commented Jul 7, 2026

Copy link
Copy Markdown
Member

Is this something we want to support? We don't support this pattern anywhere else. Wouldn't it make more sense to just throw an error?

Actually yes, it would be.

@Pyker Pyker force-pushed the fix/cross-realm-formdata-request-body branch from 5faa679 to 1bb4878 Compare July 7, 2026 15:55
request() accepts FormData bodies through the duck-typed
util.isFormDataLike() check, but only undici's own FormData class can
be encoded: the multipart extraction is gated on the webidl brand
check, so a FormData from another realm (Node.js' builtin
globalThis.FormData, another undici copy, a polyfill) fell through and
was dispatched as a body that never produced any bytes, hanging the
request until an external timeout with zero bytes on the wire.

Per review, reject such bodies instead of encoding them: the Request
constructor now throws InvalidArgumentError when a FormData-like body
is not an instance of undici's FormData, in the same place other
invalid body types are rejected, so both the HTTP/1 and HTTP/2 clients
reject cleanly at dispatch time.

Note this is a behavior change relative to v6, which encoded
cross-realm FormData successfully; v7.0.0 through 7.19.x failed these
bodies with an unrelated TypeError and 7.20.0+ hung.

Fixes: nodejs#5520
Signed-off-by: Pedro Cunha <pedroagracio@gmail.com>
@Pyker Pyker force-pushed the fix/cross-realm-formdata-request-body branch from 1bb4878 to 5a0d56a Compare July 7, 2026 15:55
@Pyker Pyker changed the title fix: encode cross-realm FormData request bodies fix: reject cross-realm FormData request bodies Jul 7, 2026
@Pyker

Pyker commented Jul 7, 2026

Copy link
Copy Markdown
Author

Reworked to reject, per the discussion: the Request constructor now throws InvalidArgumentError for FormData-like bodies that are not undici's own FormData, alongside the existing invalid-body rejections, so h1 and h2 both fail fast at dispatch time. The encode helper is gone and web/fetch is untouched (the brand check is lazy-loaded). One note for future bisecters: v6 encoded these successfully, so this formalizes the stricter v7+ behavior with a clear error instead of the hang.

@Pyker Pyker requested a review from mcollina July 7, 2026 16:08
Comment thread lib/core/request.js Outdated
Comment thread lib/core/request.js Outdated
Comment thread test/issue-5520.js Outdated
Comment thread test/issue-5520.js Outdated
Comment thread test/issue-5520.js Outdated
Comment thread test/issue-5520.js Outdated
Comment thread test/issue-5520.js Outdated
Drop the lazy-loading and rationale comments (the error message and
issue link carry the context), the tautological precondition assert,
and the tests already covered elsewhere: own-FormData encoding is
tested by test/client-request.js and test/http2-body.js, and the
string-only case is identical to the file case under rejection.

Signed-off-by: Pedro Cunha <pedroagracio@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants