Skip to content

Fix multipart limit violations being silently swallowed#6395

Open
Chaoran-Huang wants to merge 1 commit into
Effect-TS:mainfrom
Chaoran-Huang:fix/multipart-limit-swallowed
Open

Fix multipart limit violations being silently swallowed#6395
Chaoran-Huang wants to merge 1 commit into
Effect-TS:mainfrom
Chaoran-Huang:fix/multipart-limit-swallowed

Conversation

@Chaoran-Huang

Copy link
Copy Markdown

Type of change

  • Bug fix

Description

Fixes #6392.

Multipart.makeChannel (packages/effect/src/unstable/http/Multipart.ts) stores the parser's failure and its end-of-parse signal in the same exit slot. multipasta keeps running after it signals a limit/parse error and always calls onDone at the end, and the terminal onDone handler unconditionally overwrote a previously captured failure with the completion sentinel:

onError(error_) {
  exit = Option.some(Exit.fail(convertError(error_)))
},
onDone() {
  exit = Option.some(Exit.fail(Cause.Done())) // clobbers a captured failure
}

The consuming loop only inspects exit once partsBuffer is drained — which happens after the pump pull that calls parser.end() (→ onDone). Whenever the violation and end-of-input are reached in the same delivery — always the case when the body arrives in a single chunk, i.e. essentially every small upload — the captured MultipartError is overwritten by Cause.Done() and the stream completes normally. In practice this means maxParts, maxFileSize, and maxFieldSize are silently not enforced (maxTotalSize still trips because the body stream's MaxBodySize aborts it independently, before onDone).

Fix

onDone now records normal completion only when no failure was already captured:

onDone() {
  if (Option.isNone(exit)) {
    exit = Option.some(Exit.fail(Cause.Done()))
  }
}

onDone is multipasta's terminal callback (fires once, after any onError), so first-signal-wins is the correct precedence.

Test

Adds a regression test that feeds a 3-part body as a single chunk under maxParts: 2 and asserts the stream fails with TooManyParts. Verified it fails before the fix (the stream wrongly completes) and passes after — the two pre-existing Multipart tests remain green (vitest run test/unstable/http/Multipart.test.ts: 3 passed).

Note on affected versions

Reported against the published @effect/platform (0.94.5 and 0.97.0), where the same clobber uses Exit.void; on main the module has moved to effect/unstable/http and the sentinel is Cause.Done(), but the bug and fix are identical. Maintainers may want to backport to the @effect/platform line.

🤖 Generated with Claude Code

`Multipart.makeChannel` stored the parser's failure and its end-of-parse
signal in the same slot, and the terminal `onDone` callback
unconditionally overwrote a captured failure with the completion
sentinel (`Cause.Done`). The parser keeps running after signalling a
limit/parse error and always calls `onDone` at the end, so any violation
reached in the same pump as end-of-input was lost — which is always the
case when the body arrives in a single chunk. `maxParts` / `maxFileSize`
/ `maxFieldSize` limits were therefore not enforced and the stream
completed normally instead of failing.

`onDone` now records completion only when no failure was already
captured.

Adds a regression test that feeds a 3-part body as a single chunk under
`maxParts: 2` and asserts the stream fails with `TooManyParts`.

Fixes Effect-TS#6392

Co-Authored-By: Claude Fable 5 <[email protected]>
@github-project-automation github-project-automation Bot moved this to Discussion Ongoing in PR Backlog Jul 14, 2026
@changeset-bot

changeset-bot Bot commented Jul 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1e09df5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 27 packages
Name Type
effect Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-node-shared Patch
@effect/platform-node Patch
@effect/vitest Patch
@effect/ai-anthropic Patch
@effect/ai-openai-compat Patch
@effect/ai-openai Patch
@effect/ai-openrouter Patch
@effect/atom-react Patch
@effect/atom-solid Patch
@effect/atom-vue Patch
@effect/sql-clickhouse Patch
@effect/sql-d1 Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pg Patch
@effect/sql-pglite Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-do Patch
@effect/sql-sqlite-node Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch
@effect/openapi-generator Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Chaoran-Huang Chaoran-Huang marked this pull request as ready for review July 14, 2026 20:33
@IMax153 IMax153 requested a review from tim-smart July 14, 2026 21:07
@IMax153 IMax153 added bug Something isn't working 4.0 labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.0 bug Something isn't working

Projects

Status: Discussion Ongoing

Development

Successfully merging this pull request may close these issues.

Multipart: limit violations are silently swallowed when the parse completes in the same pump (onDone clobbers the onError exit)

2 participants