From f82d4ba508ce6d407e97f666974d33b202f6633e Mon Sep 17 00:00:00 2001 From: rejifald Date: Fri, 7 Aug 2026 18:50:08 +0300 Subject: [PATCH 1/2] fix(core): a {value, type} object is no longer encoded as a file (#701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isFileWrapper` treated any object carrying a `type` key as a multipart file wrapper, so a plain domain object — `{ value: 100, type: 'refund' }` — was encoded as one tiny Blob and its siblings silently dropped. The call still returned 200. On a money path that is data loss with no signal: the server sees a 3-byte part named `refund`, typed `refund`, and never learns the currency. Under `json` nesting it is worse — the whole object hoists out and the `payload` part arrives as `{}`. The guard the comment above the predicate describes was written for exactly this harm — it already names `{ value: 100, currency: 'USD' }` — but `type` was left in the disjunction, and `type` is far too ordinary a domain key to prove file-ness. It is a *modifier* on a part that is already a file (it sets the part's content type, which `appendFilePart` still honours), never the thing that makes one. A wrapper is now a file only when `value` is binary or an explicit `filename` names the part. The binary arm widens from `Uint8Array` to `ArrayBuffer.isView` in the same edit, and that is what keeps the narrowing honest: without it, `{ value: new Float32Array(…), type: 'audio/pcm' }` would have silently demoted from a file part to a nested object — trading one instance of this bug for another. `Blob` already covers `File` and `Buffer` is a `Uint8Array`; the view test adds the remaining TypedArrays and `DataView`. BREAKING CHANGE: this narrows a detection predicate, and the blast radius is exactly one shape — a wrapper with a NON-binary `value`, a `type`, and no `filename`. `{ value: csv, type: 'text/csv' }` was a documented way to give a text part an explicit content type (the upload blog names it), and it now encodes as `k[value]` + `k[type]` string parts instead. A binary `value` carrying a `type` is unaffected, as is anything with a `filename`. Migration is one key: add a `filename`, or pass `new Blob([value], { type })`. Shipped now rather than deferred because the package is at 1.0.0-rc.7 and the alternative is cutting 1.0.0 with silent data loss on a money path. The CHANGELOG entry sits under `### Changed`, marked BREAKING, for the same reason — calling it a bare `fix` would hide the one shape that moves. Three tests land in multipart-nesting.spec.ts beside the `{ value, currency }` case this one slipped past: the domain object keeps every sibling, a binary wrapper carrying only a `type` is still a file part with its content type intact, and the `filename` arm still hoists a file under `json` nesting. Refs #701 — §1, §3, §4 and §5 are still live. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 9 +++ packages/core/src/http-adapter.ts | 25 ++++--- packages/core/test/multipart-nesting.spec.ts | 77 ++++++++++++++++++++ 3 files changed, 101 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbc0b623..9c6463ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -564,6 +564,15 @@ npm release are grouped under the in-development version that introduced them. **Migration:** delete the field. No runtime behaviour changed — the stitch was already a download. To actually get another surface, use a plain `stitch({ kind })`. +- **BREAKING CHANGE: a multipart `type` no longer makes a value a file part.** + ([#701](https://github.com/rejifald/StitchAPI/issues/701)) `isFileWrapper` accepted any object + carrying a `type` key, so a domain object like `{ value: 100, type: 'refund' }` was encoded as a + tiny Blob and its **siblings silently dropped** — a `200` with the money fields gone. A wrapper is + now a file only when `value` is binary (Blob/File/ArrayBuffer/TypedArray/Buffer) or an explicit + `filename` names the part; `type` still sets a file part's content type, it just no longer creates + one. **Migration:** a non-binary `{ value, type }` upload needs a `filename`, or pass + `new Blob([value], { type })`. + ### Fixed - **Adding `cache: { ttl }` no longer turns a handled vendor failure into a process exit.** diff --git a/packages/core/src/http-adapter.ts b/packages/core/src/http-adapter.ts index 1318535e..6732fb66 100644 --- a/packages/core/src/http-adapter.ts +++ b/packages/core/src/http-adapter.ts @@ -384,20 +384,25 @@ export function decodeResponseBody( // A value that becomes a binary file part: a Blob, a raw byte view, or a // { value, filename?, type? } file wrapper. Anything else is a nested object/scalar. // -// The wrapper is recognised ONLY when it is actually file-ish: `value` is binary (a Blob / -// Uint8Array / ArrayBuffer), OR an explicit `filename`/`type` marks it a file part (so -// `{ value: 'text', filename: 'note.txt' }` is still a named text part). A plain domain object that -// merely HAPPENS to carry a `value` key — e.g. `{ value: 100, currency: 'USD' }` — is NOT a file: -// treating it as one encoded `value` as a tiny Blob and silently DROPPED its siblings. Such an -// object falls through here and recurses as a normal nested object instead. +// The wrapper is recognised ONLY when it is actually file-ish: `value` is binary (a Blob/File, an +// ArrayBuffer, or any view over one — TypedArray, Node `Buffer`, DataView), OR an explicit +// `filename` names the part (so `{ value: 'text', filename: 'note.txt' }` is still a named text +// part). A plain domain object that merely HAPPENS to carry a `value` key — e.g. +// `{ value: 100, currency: 'USD' }` — is NOT a file: treating it as one encoded `value` as a tiny +// Blob and silently DROPPED its siblings. Such an object falls through here and recurses as a +// normal nested object instead. +// +// `type` is deliberately NOT a discriminator (#701 §2). It is a *modifier* — it sets an already-file +// part's content type, and `appendFilePart` still honours it — but it is far too ordinary a domain +// key to prove file-ness on its own: `{ value: 100, type: 'refund' }` is money, not an upload, and +// admitting it here reintroduced exactly the sibling loss the paragraph above exists to prevent. function isFileWrapper(v: object): boolean { - const w = v as { value?: unknown; filename?: unknown; type?: unknown }; + const w = v as { value?: unknown; filename?: unknown }; return ( w.value instanceof Blob || - w.value instanceof Uint8Array || w.value instanceof ArrayBuffer || - w.filename !== undefined || - w.type !== undefined + ArrayBuffer.isView(w.value) || + w.filename !== undefined ); } diff --git a/packages/core/test/multipart-nesting.spec.ts b/packages/core/test/multipart-nesting.spec.ts index 919aea8a..50e897b9 100644 --- a/packages/core/test/multipart-nesting.spec.ts +++ b/packages/core/test/multipart-nesting.spec.ts @@ -170,6 +170,83 @@ describe('multipart nesting (ADR 0005 Decision 6)', () => { expect(raw).not.toContain('filename='); }); + // #701 §2: the same silent-sibling-loss bug, reached through `type` instead of `value`. + // `type` is a modifier on a file part (it sets the part's content type), never the thing that + // MAKES one — it is far too common a domain key (`{ value, type: 'refund' }`) to discriminate on. + test('a { value, type } domain object is a nested object, not a file (every sibling survives)', async () => { + server.route('POST', '/u', { body: { ok: true } }); + const upload = stitch({ + method: 'POST', + baseUrl: server.url, + path: '/u', + wire: { body: 'multipart' }, + }); + + await upload({ + body: { refund: { value: 100, type: 'refund', currency: 'USD' } }, + }); + + const raw = rawOf('/u'); + // All three fields survive as normal string parts… + expect(raw).toContain('name="refund[value]"'); + expect(raw).toContain('100'); + expect(raw).toContain('name="refund[type]"'); + expect(raw).toContain('refund'); + expect(raw).toContain('name="refund[currency]"'); + expect(raw).toContain('USD'); + // …and `refund` is NOT a file part (the bug encoded it as a 3-byte Blob typed `refund`, + // losing `type` and `currency` outright, and the call still returned 200). + expect(raw).not.toContain('name="refund"\r\n'); + expect(raw).not.toContain('filename='); + }); + + test('a binary wrapper carrying only a `type` (no filename) is still a file part', async () => { + server.route('POST', '/u', { body: { ok: true } }); + const upload = stitch({ + method: 'POST', + baseUrl: server.url, + path: '/u', + wire: { body: 'multipart' }, + }); + + await upload({ + body: { doc: { value: bytes, type: 'application/x-custom' } }, + }); + + const raw = rawOf('/u'); + expect(raw).toContain('name="doc"'); + // the `type` still reaches the wire as the part's content type… + expect(raw).toContain('application/x-custom'); + // …and it did NOT recurse into value/type string fields + expect(raw).not.toContain('doc[value]'); + expect(raw).not.toContain('doc[type]'); + }); + + test("'json' nesting: an explicit { value, filename } still hoists to a file part", async () => { + server.route('POST', '/u', { body: { ok: true } }); + const upload = stitch({ + method: 'POST', + baseUrl: server.url, + path: '/u', + wire: { body: 'multipart', multipart: { nesting: 'json' } }, + }); + + await upload({ + body: { + amount: { value: 100, type: 'refund' }, + note: { value: 'hello', filename: 'n.txt' }, + }, + }); + + const raw = rawOf('/u'); + // the filename arm survives the narrowing — still hoisted out of the JSON part + expect(raw).toContain('name="note"'); + expect(raw).toContain('filename="n.txt"'); + // the domain object rides inside the JSON part, both keys intact + expect(raw).toContain('name="payload"'); + expect(raw).toContain('"amount":{"value":100,"type":"refund"}'); + }); + test('a real { value: , filename } wrapper is still a file part', async () => { server.route('POST', '/u', { body: { ok: true } }); const upload = stitch({ From c2a43ffc5edaa5fac6625e9c247fa2c7e6044ea5 Mon Sep 17 00:00:00 2001 From: rejifald Date: Fri, 7 Aug 2026 19:05:21 +0300 Subject: [PATCH 2/2] fix(core): drop the TypedArray widening from the wrapper predicate (#701) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit paired the `type` narrowing with widening the binary arm from `Uint8Array` to `ArrayBuffer.isView`, and justified it as the thing that "keeps the narrowing honest". Measured against the size gate, that justification does not survive. `check:size` is a CI job and the core entry's budget has almost nothing left: `origin/main` measures 24664 B gzip against a 24678.4 B budget — 14.4 bytes. The widened predicate came in at 24671 B (7.4 B left), so it spent half the remaining margin of a budget shared with every other core PR in flight. Reordering the disjunction recovered 2 B and hoisting `w.value` to a local recovered none; the cost is irreducible because `ArrayBuffer.isView(` is a token the bundle does not otherwise contain. Dropping it lands at 24658 B — 6 bytes BELOW main, so this PR now gives the budget back instead of eating it. The correctness argument was also overstated. I claimed an exotic view in a wrapper would "trade one instance of this bug for another"; it does not. `{ value: new Float32Array([1, 2]), type: 'audio/pcm' }` recurses to `k[value][0]=1`, `k[value][1]=2`, `k[type]=audio/pcm` — every byte still on the wire, in a shape the server rejects. That is a loud 400, not the silent 200-with-fields-missing this predicate exists to prevent. The two are not the same harm class, and only the second one justifies spending a contended budget. So the predicate is now exactly the original minus `type`: Blob/File, Uint8Array/Buffer, ArrayBuffer, or an explicit `filename`. A condition changed rather than a condition added. The exotic-view gap is real and stays real — `appendFilePart` would already encode a `DataView` correctly, only the detector is narrow. It is recorded in the comment block above the predicate and tracked separately rather than smuggled in here. No test changes: the binary-wrapper test uses a `Uint8Array`, which both spellings accept. CHANGELOG migration note now names the binary types exactly instead of saying "TypedArray". Gates: lint, types, 1494 tests, changelog, contract, unknown-keys all green; `check:size` 24.08/24.10 KB whole entry and 21.50/21.55 KB for `import { stitch }`, both with more headroom than main. Refs #701 — §1, §3, §4 and §5 are still live. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 +++--- packages/core/src/http-adapter.ts | 21 +++++++++++++-------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c6463ef..233d0431 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -568,10 +568,10 @@ npm release are grouped under the in-development version that introduced them. ([#701](https://github.com/rejifald/StitchAPI/issues/701)) `isFileWrapper` accepted any object carrying a `type` key, so a domain object like `{ value: 100, type: 'refund' }` was encoded as a tiny Blob and its **siblings silently dropped** — a `200` with the money fields gone. A wrapper is - now a file only when `value` is binary (Blob/File/ArrayBuffer/TypedArray/Buffer) or an explicit + now a file only when `value` is binary (Blob/File/Uint8Array/Buffer/ArrayBuffer) or an explicit `filename` names the part; `type` still sets a file part's content type, it just no longer creates - one. **Migration:** a non-binary `{ value, type }` upload needs a `filename`, or pass - `new Blob([value], { type })`. + one. **Migration:** a `{ value, type }` part whose `value` is not one of those binary types needs a + `filename`, or pass `new Blob([value], { type })`. ### Fixed diff --git a/packages/core/src/http-adapter.ts b/packages/core/src/http-adapter.ts index 6732fb66..b15c03c8 100644 --- a/packages/core/src/http-adapter.ts +++ b/packages/core/src/http-adapter.ts @@ -384,24 +384,29 @@ export function decodeResponseBody( // A value that becomes a binary file part: a Blob, a raw byte view, or a // { value, filename?, type? } file wrapper. Anything else is a nested object/scalar. // -// The wrapper is recognised ONLY when it is actually file-ish: `value` is binary (a Blob/File, an -// ArrayBuffer, or any view over one — TypedArray, Node `Buffer`, DataView), OR an explicit -// `filename` names the part (so `{ value: 'text', filename: 'note.txt' }` is still a named text -// part). A plain domain object that merely HAPPENS to carry a `value` key — e.g. -// `{ value: 100, currency: 'USD' }` — is NOT a file: treating it as one encoded `value` as a tiny -// Blob and silently DROPPED its siblings. Such an object falls through here and recurses as a -// normal nested object instead. +// The wrapper is recognised ONLY when it is actually file-ish: `value` is binary (a Blob/File, a +// Uint8Array/Buffer, or an ArrayBuffer), OR an explicit `filename` names the part (so +// `{ value: 'text', filename: 'note.txt' }` is still a named text part). A plain domain object that +// merely HAPPENS to carry a `value` key — e.g. `{ value: 100, currency: 'USD' }` — is NOT a file: +// treating it as one encoded `value` as a tiny Blob and silently DROPPED its siblings. Such an +// object falls through here and recurses as a normal nested object instead. // // `type` is deliberately NOT a discriminator (#701 §2). It is a *modifier* — it sets an already-file // part's content type, and `appendFilePart` still honours it — but it is far too ordinary a domain // key to prove file-ness on its own: `{ value: 100, type: 'refund' }` is money, not an upload, and // admitting it here reintroduced exactly the sibling loss the paragraph above exists to prevent. +// +// The binary arm is the three concrete types above and NOT `ArrayBuffer.isView`, so an exotic view +// (`Float32Array`, `DataView`) in a wrapper is not a file part. That is a real gap, tracked +// separately — widening it here costs ~11 gzip bytes of a budget with ~14 left, and the failure it +// prevents is a mis-shaped body the server rejects (`k[value][0]`, `k[value][1]`, …), not the +// silent sibling loss this predicate exists to stop. function isFileWrapper(v: object): boolean { const w = v as { value?: unknown; filename?: unknown }; return ( w.value instanceof Blob || + w.value instanceof Uint8Array || w.value instanceof ArrayBuffer || - ArrayBuffer.isView(w.value) || w.filename !== undefined ); }