Skip to content

feat(readBody): support formdata type#1164

Merged
pi0 merged 6 commits into
h3js:mainfrom
huseeiin:multipart-form-data
Jul 6, 2026
Merged

feat(readBody): support formdata type#1164
pi0 merged 6 commits into
h3js:mainfrom
huseeiin:multipart-form-data

Conversation

@huseeiin

@huseeiin huseeiin commented Jul 18, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added an optional options parameter to request body parsing, including type: "formData" for multipart/form-data.
    • Exposed a ReadBodyOptions type for the new API.
  • Bug Fixes

    • Default parsing remains JSON, with 400 Bad Request for invalid JSON.
    • URL-encoded bodies are parsed from application/x-www-form-urlencoded (or when explicitly requested).
    • Empty bodies still return undefined; repeated fields are preserved as arrays.
  • Documentation

    • Updated readBody documentation and examples to reflect the default JSON behavior and explicit multipart opt-in.
  • Tests

    • Added multipart/form-data tests (valid parsing, missing opt-in behavior, and malformed payload errors).

@huseeiin huseeiin requested a review from pi0 as a code owner July 18, 2025 09:38
@codecov

codecov Bot commented Jul 18, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread src/utils/body.ts Outdated
const text = await event.req.text();
const contentType = event.req.headers.get("content-type") || "";

if (contentType.startsWith("multipart/form-data"))

@pi0 pi0 Jul 18, 2025

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.

We should make it opt-in by supporting event, opts?: { type: "json" | formData" } and throw an HTTPError if it is not.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We should make it opt-in by supporting event, opts?: { type: "json" | formData" } and throw an HTTPError if it is not.

i support this. but my changes remain valid, right? since opts is optional. when you specify type in opts, you're specifying a certain body type, when you're not, you're parsing any body as an object or string!

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.

As security measure we should make types opt-in other than JSON (so if no options only json)

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.

Context: #875

@pi0 pi0 changed the title check for multipart/form-data feat(readBody): support formdata type Jul 18, 2025
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dff867b9-0d02-458c-96af-9796e3deffb4

📥 Commits

Reviewing files that changed from the base of the PR and between 0f31cad and 0198322.

📒 Files selected for processing (4)
  • docs/2.utils/1.request.md
  • src/index.ts
  • src/utils/body.ts
  • test/body.test.ts
✅ Files skipped from review due to trivial changes (1)
  • src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • docs/2.utils/1.request.md
  • src/utils/body.ts
  • test/body.test.ts

📝 Walkthrough

Walkthrough

readBody now accepts an options parameter and can explicitly parse multipart form-data via event.req.formData(). Shared helpers collect repeated fields into arrays, the request docs reflect the new parsing contract, and multipart behavior is covered by tests.

Changes

readBody multipart handling

Layer / File(s) Summary
Body parsing helpers and readBody branch
src/utils/body.ts, src/utils/internal/body.ts, src/index.ts
readBody adds an options.type branch for formData, reads content-type up front, keeps text/URL-encoded/JSON handling, and uses shared helpers to turn URL-encoded and FormData entries into objects with repeated keys preserved as arrays. The public re-export surface also includes ReadBodyOptions.
Request docs update
docs/2.utils/1.request.md
The readBody docs update the signature, parsing rules, and examples for default and explicit body-type parsing.
Multipart body tests
test/body.test.ts
New multipart tests cover explicit form-data parsing, the non-opt-in 400 response, and malformed multipart payloads.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

A rabbit read the body twice,
With form-data parsed just right.
Repeated fields stayed in a row,
And docs and tests now tell the show.
🐇📄✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding opt-in formData support to readBody.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/utils/body.ts`:
- Around line 30-31: The multipart form parsing branch in the body utility is
missing the same error handling used by the JSON path. Update the body parsing
logic around the event.req.formData() call to wrap malformed/truncated multipart
failures in a try/catch and throw a consistent HTTPError 400, matching the
existing JSON handling in the same helper so bad payloads don’t surface as
unhandled errors.
- Around line 30-31: The multipart parsing in body handling is collapsing
repeated field names because Object.fromEntries on event.req.formData() keeps
only the last value. Update the logic in the body utility to preserve duplicate
multipart entries instead of flattening them, and adjust the return shape
accordingly so repeated keys from formData are retained.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a8729cc8-0a87-4e3b-b638-9d9274acb40a

📥 Commits

Reviewing files that changed from the base of the PR and between 1ff4999 and 8654ef4.

📒 Files selected for processing (1)
  • src/utils/body.ts

Comment thread src/utils/body.ts Outdated
@pi0x

pi0x commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Pushed e2e146c reworking this toward the opt-in design discussed above (#875).

What changed

  • readBody(event, { type })formData is now strictly opt-in and never auto-detected from Content-Type, so an untrusted request can no longer push readBody into multipart parsing on its own. Default JSON and existing x-www-form-urlencoded auto-parse are unchanged (no breaking change).
  • Malformed multipart now throws a clean HTTPError(400), matching the JSON path (addresses CodeRabbit's error-handling comment).
  • Repeated form fields are preserved as arrays instead of collapsing to the last value — shared with url-encoded parsing (addresses CodeRabbit's duplicate-field comment).
  • Added readBody-based multipart tests (opt-in parse, no auto-parse without opt-in, malformed → 400), running in both web and node.

Open question: the thread said "if no options only json" — I kept the existing x-www-form-urlencoded auto-parse by default to avoid a breaking change beyond this PR. If you want strict JSON-only default, it's a one-line change plus updating the one urlencoded test to pass { type: "urlencoded" }.

🤖 This change was drafted with Claude Code (Opus 4.8) and reviewed by me before pushing.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/2.utils/1.request.md`:
- Line 28: The `readBody` documentation heading is malformed and the
`options.type` union is truncated, so fix the signature text in the
`readBody(event, options?: ...)` heading to use matching braces/parens and
include the full `type` union. Update the affected markdown heading so the
function signature is syntactically correct and reflects the complete allowed
`type` values.

In `@src/utils/internal/body.ts`:
- Around line 4-10: Both exported helpers in body.ts are missing explicit return
type annotations, which breaks declaration emit under isolatedDeclarations.
Update parseURLEncodedBody and parseFormData to declare their return types
directly, using the same type that collectEntries produces, so the exported
signatures are fully explicit and stable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f61cafa7-57aa-499d-8eee-013b53c0923b

📥 Commits

Reviewing files that changed from the base of the PR and between 8654ef4 and 5f640f1.

📒 Files selected for processing (4)
  • docs/2.utils/1.request.md
  • src/utils/body.ts
  • src/utils/internal/body.ts
  • test/body.test.ts

Comment thread docs/2.utils/1.request.md Outdated
```

### `readBody(event)`
### `readBody(event, options?: { type?: "json")`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix malformed function-signature heading.

Braces/parens are mismatched and the type union is truncated to "json" only.

📝 Proposed fix
-### `readBody(event, options?: { type?: "json")`
+### `readBody(event, options?: { type?: "json" | "text" | "urlencoded" | "formData" })`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### `readBody(event, options?: { type?: "json")`
### `readBody(event, options?: { type?: "json" | "text" | "urlencoded" | "formData" })`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/2.utils/1.request.md` at line 28, The `readBody` documentation heading
is malformed and the `options.type` union is truncated, so fix the signature
text in the `readBody(event, options?: ...)` heading to use matching
braces/parens and include the full `type` union. Update the affected markdown
heading so the function signature is syntactically correct and reflects the
complete allowed `type` values.

Comment thread src/utils/internal/body.ts Outdated
Multipart form-data was previously parsed automatically based on the
`Content-Type` header. Per h3js#875 this is now strictly opt-in via
`readBody(event, { type: "formData" })` so an untrusted request can't
drive readBody into (potentially expensive) multipart parsing.

Also:
- wrap `formData()` in try/catch -> HTTPError 400, matching the JSON path
- preserve repeated form fields as arrays instead of dropping earlier
  values (shared with url-encoded parsing)
- add readBody-based multipart tests (opt-in, no auto-parse, malformed 400)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@pi0x pi0x force-pushed the multipart-form-data branch from 5f640f1 to 0f31cad Compare July 5, 2026 20:23
pi0 and others added 3 commits July 5, 2026 21:37
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-type tests

- `type: "text"` now returns an empty string for an empty body instead of
  `undefined`, matching the raw-string contract.
- Extract a named `ReadBodyOptions` type (exported) so automd renders a
  clean `readBody(event, options?)` heading instead of mangling the inline
  string-literal union.
- Add tests for the explicit `text`, `urlencoded`, and `json` modes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@pi0 pi0 merged commit e12c0e8 into h3js:main Jul 6, 2026
7 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