diff --git a/CHANGELOG.md b/CHANGELOG.md index cbc0b623..233d0431 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/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 `{ value, type }` part whose `value` is not one of those binary types 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..b15c03c8 100644 --- a/packages/core/src/http-adapter.ts +++ b/packages/core/src/http-adapter.ts @@ -384,20 +384,30 @@ 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 +// 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; 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 + 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({