Vendor multipasta as MultipartParser - #7012
Conversation
🦋 Changeset detectedLatest commit: bd8ed2c The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
This PR vendors multipasta v0.2.8 into packages/effect/src/unstable/http/Multipasta and switches the existing unstable HTTP multipart consumers (Multipart.ts, NodeMultipart.ts, BrowserHttpClient.ts) from the external package to the internal copy. It removes the multipasta dependency from packages/effect/package.json and packages/platform-browser/package.json (including the lockfile), preserves the Node/Web/headers-parser subpath exports, and ports the upstream multipart test suite into packages/effect/test/unstable/http/Multipasta.test.ts.
- package.json / lockfile cleanup: removes external
multipastaentries and includes the internal LICENSE in published files. - New vendored modules: content-type parser, header parser, Boyer-Moore-Horspool search, multipart core, plus Node stream and Web stream adapters.
- Consumer migration:
Multipart.ts,NodeMultipart.ts, andBrowserHttpClient.tsnow import fromeffect/unstable/http/Multipasta*paths. - Test port: adds 98 tests covering fields, files, limits, chunked parsing, empty parts, and random-data smoke testing.
Validation run: packages/effect/test/unstable/http/Multipasta.test.ts and Multipart.test.ts pass; type checks pass for effect, @effect/platform-node, and @effect/platform-browser.
@v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
tim-smart
changed the title
There was a problem hiding this comment.
Important
Two process/licensing concerns need resolution before this lands: the vendored multipasta code has lost its MIT attribution, and the public module changes need a changeset.
Reviewed changes
This incremental review covers commit a0f89f8, which renamed the vendored multipart parser from Multipasta* to MultipartParser*, flattened the internal API surface, moved the Node stream adapter into @effect/platform-node/NodeMultipartParser, and removed the old Multipasta re-export modules. It also switched the random-data smoke test to a deterministic XOR-shift PRNG and removed the fail-data workspace artifact. Two prior Pullfrog review threads were addressed by this commit and have been resolved.
- Module rename and flattening: public types moved from
MultipartParser/internal/index.tsetc. directly intoMultipartParser.ts,HeadersParser.ts,Search.ts, andWeb.ts; intermediateheadersParserApi.ts,index.ts,searchApi.ts,node.ts, andweb.tsfiles were deleted. - Node adapter relocation:
NodeMultipart.tsnow imports from local./NodeMultipartParser.ts, which is also re-exported fromplatform-node/src/index.ts. - Consumer import updates:
Multipart.tsandBrowserHttpClient.tsupdated toeffect/unstable/http/MultipartParser*paths. - Test cleanup:
packages/effect/test/unstable/http/Multipasta.test.tswas replaced bypackages/platform-node/test/MultipartParser.test.tswith a seeded PRNG and nofail-datafile write. - Dependency removal:
multipastaremoved frompackages/effect/package.json,packages/platform-browser/package.json, andpnpm-lock.yaml.
⚠️ Missing multipasta attribution after vendor
The second commit removed the // Vendored from multipasta v0.2.8. Copyright (c) 2023-present The Contributors. MIT licensed. headers from every parser source file and dropped src/unstable/http/Multipasta/internal/LICENSE from packages/effect/package.json's files array. The code in those files is still a direct derivative of the vendored multipasta implementation, so the MIT license requires preserving the copyright notice and permission notice. Restore the per-file vendored headers and include the LICENSE in published artifacts, or document an explicit re-licensing if all copyright holders have agreed.
Technical details
# Missing multipasta attribution after vendor
## Affected sites
- `packages/effect/package.json:58` — `files` array no longer includes the multipasta LICENSE.
- `packages/effect/src/unstable/http/MultipartParser/internal/headers.ts` — vendored header comment removed.
- `packages/effect/src/unstable/http/MultipartParser/internal/multipart.ts` — vendored header comment removed.
- `packages/effect/src/unstable/http/MultipartParser/internal/search.ts` — vendored header comment removed.
- `packages/effect/src/unstable/http/MultipartParser/Web.ts` — vendored header comment removed.
- `packages/platform-node/src/NodeMultipartParser.ts` — vendored header comment removed.
## Required outcome
- Published `effect` package includes the multipasta MIT license and copyright notice.
- Every file that remains substantially derived from multipasta carries a clear attribution header.
## Suggested approach
- Re-add `src/unstable/http/MultipartParser/internal/LICENSE` to `package.json#files`.
- Restore `// Vendored from multipasta v0.2.8. Copyright (c) 2023-present The Contributors. MIT licensed.` headers (or equivalent) to the derived files.⚠️ Changeset needed for new public modules
The PR introduces new public entry points (effect/unstable/http/MultipartParser, effect/unstable/http/MultipartParser/HeadersParser, effect/unstable/http/MultipartParser/Search, effect/unstable/http/MultipartParser/Web, and @effect/platform-node/NodeMultipartParser) and removes the previous Multipasta* paths and multipasta dependency. These are exported API/runtime changes and should be recorded in .changeset/ so the release notes capture the break/replacement.
Technical details
# Changeset needed for new public modules
## Affected sites
- `packages/effect/package.json` — `multipasta` dependency removed.
- `packages/effect/src/unstable/http/index.ts` — new `MultipartParser` barrel export.
- `packages/platform-node/src/index.ts` — new `NodeMultipartParser` export.
## Required outcome
- A changeset file describes the removal of `effect/unstable/http/Multipasta*`, the addition of `effect/unstable/http/MultipartParser*`, and the relocation of the Node adapter to `@effect/platform-node/NodeMultipartParser`.@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
…6c284172 # Conflicts: # packages/effect/package.json # pnpm-lock.yaml
There was a problem hiding this comment.
Important
The latest commits clean up the unused Web Streams adapter, but the two process-blocking concerns from the prior review remain unaddressed: the multipasta-derived code still lacks MIT attribution, and there is still no changeset for the public module rename/relocation.
Reviewed changes
This incremental review covers the commits since a0f89f8: 171d889f removed the unused effect/unstable/http/MultipartParser/Web.ts adapter and its tests, and a9ed41c04 merged origin/main (resolving packages/effect/package.json and pnpm-lock.yaml conflicts from the INI/YAML/TOML parser inline PR). The migration guide was updated to drop the old Multipasta* entries and add MultipartParser* plus @effect/platform-node/NodeMultipartParser.
- Removed Web adapter:
packages/effect/src/unstable/http/MultipartParser/Web.tsand the correspondingdescribe("web api")block inpackages/platform-node/test/MultipartParser.test.tswere deleted. - Merge cleanup:
packages/effect/package.jsonandpnpm-lock.yamlwere reconciled with the newfind-my-way-tsdependency and the removal ofmultipasta. - Migration doc updated:
migration/v3-to-v4.mdnow lists the new module paths and removes the oldMultipasta*paths.
⚠️ Missing multipasta attribution after vendor
The vendored multipasta code in packages/effect/src/unstable/http/MultipartParser/internal/headers.ts, multipart.ts, search.ts, and packages/platform-node/src/NodeMultipartParser.ts still has no MIT copyright/permission notice. Only contentType.ts carries an attribution (and it points to fast-content-type-parse, not multipasta). No multipasta LICENSE file is present or included in packages/effect/package.json#files.
Technical details
# Missing multipasta attribution after vendor
## Affected sites
- `packages/effect/src/unstable/http/MultipartParser/internal/headers.ts` — no vendored header.
- `packages/effect/src/unstable/http/MultipartParser/internal/multipart.ts` — no vendored header.
- `packages/effect/src/unstable/http/MultipartParser/internal/search.ts` — no vendored header.
- `packages/platform-node/src/NodeMultipartParser.ts` — no vendored header.
- `packages/effect/package.json:58` — `files` array does not include a multipasta LICENSE.
## Required outcome
- Published `effect` package includes the multipasta MIT license and copyright notice.
- Every file that remains substantially derived from multipasta carries a clear attribution header.
## Suggested approach
- Re-add the multipasta LICENSE file and include it in `package.json#files`.
- Restore per-file vendored headers on the derived files.⚠️ Changeset needed for new public modules
The PR still has no changeset describing the removal of effect/unstable/http/Multipasta*, the addition of effect/unstable/http/MultipartParser*, and the relocation of the Node adapter to @effect/platform-node/NodeMultipartParser.
Technical details
# Changeset needed for new public modules
## Affected sites
- `packages/effect/package.json` — `multipasta` dependency removed.
- `packages/effect/src/unstable/http/index.ts` — new `MultipartParser` barrel export.
- `packages/platform-node/src/index.ts` — new `NodeMultipartParser` export.
## Required outcome
- A changeset file describes the removal of `effect/unstable/http/Multipasta*`, the addition of `effect/unstable/http/MultipartParser*`, and the relocation of the Node adapter to `@effect/platform-node/NodeMultipartParser`.ℹ️ Notes
- Tests and type checks pass for the touched packages after installing the missing
find-my-way-tsdependency in the local workspace. - Removing the Web adapter is a sensible simplification since nothing in the repo currently consumes it.
@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Important
The new commits address one of the two outstanding concerns by adding a changeset, but the MIT attribution for the vendored multipasta code is still missing.
Reviewed changes
This incremental review covers commit 244730c, which added the changeset for the new public modules and removed the redundant re-exports from packages/platform-node/src/NodeMultipartParser.ts so the Node adapter no longer duplicates the low-level parser API surface.
- Changeset added:
.changeset/swift-pandas-parse.mdnow records the patch-level change acrosseffect,@effect/platform-browser, and@effect/platform-nodefor vendoringMultipartParserand removing themultipastadependency. - Node adapter cleanup:
NodeMultipartParser.tsno longer re-exportsMultipartError,PartInfo, anddecodeField;NodeMultipart.tsnow imports those directly fromeffect/unstable/http/MultipartParser.
⚠️ Missing multipasta attribution after vendor
The vendored parser source (packages/effect/src/unstable/http/MultipartParser/internal/headers.ts, multipart.ts, search.ts) and packages/platform-node/src/NodeMultipartParser.ts remain substantially derived from multipasta v0.2.8, but none of them carry a copyright or permission notice, and no multipasta LICENSE file is included in published artifacts. The changeset alone does not satisfy the MIT license condition to include the original copyright and permission notice in copies or substantial portions of the software.
Technical details
# Missing multipasta attribution after vendor
## Affected sites
- `packages/effect/src/unstable/http/MultipartParser/internal/headers.ts` — no vendored/attribution header.
- `packages/effect/src/unstable/http/MultipartParser/internal/multipart.ts` — no vendored/attribution header.
- `packages/effect/src/unstable/http/MultipartParser/internal/search.ts` — no vendored/attribution header.
- `packages/platform-node/src/NodeMultipartParser.ts` — no vendored/attribution header.
- `packages/effect/package.json:58` — `files` array does not include a multipasta LICENSE or NOTICE file.
## Required outcome
- The published `effect` package includes the multipasta MIT copyright and permission notice.
- Substantially-derived source files carry a clear attribution header pointing at the original work and license.
## Suggested approach
- Add `// Derived from multipasta (https://github.com/tim-smart/multipasta), used under the MIT license.
// Copyright (c) 2023-present The Contributors` to each derived file.
- Add a `NOTICE` or `LICENSE.multipasta` file to `packages/effect` containing the full MIT license text and include it in `package.json#files`.
Sources:
- Multipasta v0.2.8 LICENSE: https://raw.githubusercontent.com/tim-smart/multipasta/v0.2.8/LICENSE
- MIT license condition: “The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.”@v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This incremental review covers commit aec51207, which defends the filename* parser in packages/effect/src/unstable/http/MultipartParser/internal/multipart.ts against malformed percent-encoded filenames.
- Malformed
filename*handling: wrappeddecodeURIComponent(parts[1])in atry/catchand fell back to the raw encoded value when decoding fails. - Test coverage: added a
Malformed encoded filenamescase topackages/platform-node/test/MultipartParser.test.tsasserting thatfilename*=utf-8''%ZZparses with filename%ZZinstead of throwing. - Changeset: added
.changeset/tidy-carpets-smile.mdscoped toeffectfor the bugfix.
Validation run: targeted Malformed encoded filenames test passes, full MultipartParser.test.ts passes, and effect Multipart.test.ts passes. pnpm check for effect and @effect/platform-node still fails on the unrelated missing find-my-way-ts workspace dependency.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This incremental review covers commit 947d5c77, which fixes a maxPartSize accounting bug in the vendored multipart parser and adds a regression test.
- Fixed
maxPartSizeaccumulation: movedstate.partSize = 0inpackages/effect/src/unstable/http/MultipartParser/internal/multipart.tsso it resets for every new part, not only for file parts. Previously, the byte count from one field carried into the next, causingmaxPartSizeto fire on a later field that was within its own limit. - Added regression test:
packages/platform-node/test/MultipartParser.test.tsnow has aResets maxPartSize between field partscase with two 32-byte fields and a 100-bytemaxPartSize, asserting both fields parse cleanly. - Changeset already in place: the prior commit's
.changeset/tidy-carpets-smile.mdcovers multipart parsing bugfixes.
Validation run: pnpm --filter @effect/platform-node test --run test/MultipartParser.test.ts -t "Resets maxPartSize between field parts" passes.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This incremental review covers commit 5dead0f5, which fixes two multipart parser regressions introduced during the vendor and adds covering tests.
- Fixed false
EndNotReachedon successful completion: replaced thestate.state === State.bodycheck inpackages/effect/src/unstable/http/MultipartParser/internal/multipart.tswith an explicitstate.doneflag that is set only when the trailing--boundary is seen. The previous check could reportEndNotReachedafter a body completed successfully. - Stopped Node file chunks after an error: added an early return in the
NodeMultipartParser.tsfile-chunk callback whencurrentErroris set, preventing more data from being pushed into aFileStreamafter the parser has already emitted an error. - Added regression tests:
packages/platform-node/test/MultipartParser.test.tsnow covers truncated mid-header input and the "stops sending file chunks after an error" Node adapter behavior.
Validation run: packages/platform-node/test/MultipartParser.test.ts, packages/effect/test/unstable/http/Multipart.test.ts, packages/platform-node/test/NodeHttpServer.test.ts, and packages/platform-browser/test/BrowserHttpClient.test.ts all pass.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
This incremental review covers commit 74ec81db, which extended the vendored contentType.ts regexes to accept non-Latin-1 (non-BMP) characters in quoted header parameters and added an integration test for non-Latin filenames.
- Extended content-type regex range:
paramREandquotedPairREnow allow code points up toU+10FFFF, matching real-world UTF-8filenamevalues emitted by browsers. - Added integration test:
packages/effect/test/unstable/http/Multipart.test.tsnow asserts that aFormDatafile with a Japanese filename round-trips through the multipart parser.
Validation run: packages/effect/test/unstable/http/Multipart.test.ts, packages/platform-node/test/MultipartParser.test.ts, packages/platform-browser/test/BrowserHttpClient.test.ts, pnpm --filter effect check, pnpm --filter @effect/platform-node check, and pnpm lint all pass after installing the missing workspace dependency.
@v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This incremental review covers commit ad2128ed, which corrects the FileStream.filename type in @effect/platform-node/NodeMultipartParser from an unsound string to string | undefined and adds a type/runtime test verifying the behavior.
- Corrected
FileStream.filenametype:packages/platform-node/src/NodeMultipartParser.tsnow declaresreadonly filename: string | undefinedand assignsinfo.filenamedirectly, aligning with thePartInfo.filenametype. - Added coverage:
packages/platform-node/test/MultipartParser.test.tsgained anexposes missing filenames as undefinedtest that usesexpectTypeOfto assert the type and checks that a file part without afilenameparameter yieldsundefinedat runtime. - Two prior Pullfrog review threads were addressed: the
tidy-carpets-smile.mdchangeset description and the staleobs-textcomment incontentType.tswere resolved in earlier commits and are now closed.
Validation note: the PR body’s validation commands cover the affected tests and type checks.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This incremental review covers commit bd8ed2c8, which fixes final-boundary detection when the trailing -- delimiter of a multipart body is split across write() calls.
- Configurable minimum chunk length in boundary search:
packages/effect/src/unstable/http/MultipartParser/internal/search.tsnow accepts aminimumChunkLengthparameter and only buffers chunks smaller than that threshold, instead of buffering any chunk shorter than the full needle. - Multipart parser uses 2-byte threshold:
packages/effect/src/unstable/http/MultipartParser/internal/multipart.tspassesminimumChunkLength = 2to the boundary search so a chunk containing the trailing--is processed immediately rather than held until the next write. - Regression test added:
packages/effect/test/unstable/http/Multipart.test.tshas a newhandles the final boundary delimiter split between the trailing hyphenstest that writes the final boundary with the last hyphen split across two writes and asserts the parser reachesonDone.
Validation run: packages/platform-node/test/MultipartParser.test.ts passes (84 tests). The new Multipart.test.ts case could not be executed locally because find-my-way-ts is missing from the workspace; this dependency issue is unrelated to the parser changes.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
1 participant
Summary
effect/unstable/http/MultipartParser, with Search and HeadersParser subpaths@effect/platform-node/NodeMultipartParserand update all internal consumersMultipasta*module paths and all external dependency and lockfile entriesValidation
pnpm exec vitest run --project effect packages/effect/test/unstable/http/Multipart.test.tspnpm exec vitest run --project @effect/platform-node packages/platform-node/test/MultipartParser.test.ts packages/platform-node/test/NodeHttpServer.test.tspnpm exec vitest run --project @effect/platform-browser packages/platform-browser/test/BrowserHttpClient.test.tspnpm --filter effect checkpnpm --filter @effect/platform-node checkpnpm --filter @effect/platform-browser checkpnpm lintpnpm jsdocs(new modules clean; existing unrelated Schema and HttpMiddleware warnings remain)pnpm test --run packages/platform-node/test/MultipartParser.test.tspnpm checkpnpm lint-fixCloses EFF-450
Closes EFF-454