diff --git a/.changeset/24042-hono-undeclared-body-refusal.md b/.changeset/24042-hono-undeclared-body-refusal.md new file mode 100644 index 00000000000..8f9eb4e4ad7 --- /dev/null +++ b/.changeset/24042-hono-undeclared-body-refusal.md @@ -0,0 +1,30 @@ +--- +'@objectstack/hono': minor +--- + +fix(hono)!: `createHonoApp` reads a dispatched body by `@objectstack/plugin-hono-server`'s rule, so a JSON body sent as `text/plain`, or any other body that declares neither JSON nor a form, is refused `400 INVALID_REQUEST` when the dispatched domain reads it, instead of being parsed or dispatched as `{}` + +Clause-②: no (narrowing) + + + +**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes. + +The `${prefix}/*` catch-all and the `/auth/*` legacy fallback parsed every body as JSON whatever its `Content-Type`, and handed `dispatch()` `{}` for any body that did not parse. So the two Hono doors answered one request two ways: `@objectstack/plugin-hono-server` (`objectstack dev`, `os serve`) refused a valid query body sent as `text/plain`, and this door honoured it; a garbled body that did not declare JSON reached the data domain here as `{}`, the empty query, and read every row the caller could read. + +What changes for an integrator, on a host that serves `createHonoApp`, for a `POST`, `PUT` or `PATCH` body read by a domain that reads the parsed body (the data, metadata and every other domain that takes `body`), and on the `/auth/*` legacy fallback for every method it hands a body (all but `GET` and `HEAD`): + +- A JSON body sent as `text/plain`, or as any media type other than JSON or a form: FROM parsed and honoured, TO `400` with `{ "success": false, "error": { "code": "INVALID_REQUEST", "message": "The request body was sent as text/plain, which this server does not read into a request body. It was refused rather than read as an empty body: send it as application/json." } }`, the message naming the media type that was sent. +- Any other body that declares neither JSON nor a form (`text/plain`, `application/octet-stream`, a `+json` suffix type such as `application/merge-patch+json`, or no `Content-Type` at all), when the request declares a body (`Content-Length` other than `0`, or `Transfer-Encoding`): FROM `{}`, TO the same refusal ("sent with no Content-Type" when there is none). +- A `multipart/form-data` or `application/x-www-form-urlencoded` body: FROM `{}`, TO its parsed fields, as `plugin-hono-server` hands them (Hono's `parseBody`: the last value of a repeated name). A form that does not parse: TO the refusal "The request body could not be parsed as form data (it was sent as multipart/form-data). It was refused rather than read as an empty body." + +The domain acts on nothing past its read of the body, and the dispatcher's own gates still answer first. Each answer is the status and body `@objectstack/plugin-hono-server` gives the same request. This replaces the earlier `createHonoApp` entry's statement that a body that does not declare JSON is read as before. + +**Remedy, in one line.** Send a JSON body with `Content-Type: application/json`, or send no body. + +**Unchanged.** + +- A body sent as `application/json` (matched case-insensitively) is parsed when well-formed, `{}` when blank or whitespace-only, and refused when it does not parse, as before. +- A request that declares no body is dispatched as `{}`, whatever its media type. +- The request handed on beside the body still arrives unread, in every case. A domain that forwards it and reads its own bytes answers exactly as before, whatever the body declares: `/webhooks/redeliver` and the inbound automation hooks give their own refusal after their own sign-in or signature check, and the `/approvals/act` form `POST` reads its form. +- The catch-all dispatches `GET`, `HEAD` and `DELETE` with no body, and the `/auth/*` fallback hands `GET` and `HEAD` `{}`, as before. diff --git a/packages/adapters/hono/src/hono-body-refusal.test.ts b/packages/adapters/hono/src/hono-body-refusal.test.ts index 06ebc0b3e6a..b3c0adc75fb 100644 --- a/packages/adapters/hono/src/hono-body-refusal.test.ts +++ b/packages/adapters/hono/src/hono-body-refusal.test.ts @@ -1,14 +1,18 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [#23914] `createHonoApp` refuses a declared JSON body that does not parse - * with `400 INVALID_REQUEST` when a dispatched domain READS the body, and only - * then — the rule `@objectstack/plugin-hono-server` applies to `req.body` — and - * answers it with the same status and body as that package. + * [#23914, #24042] `createHonoApp` reads a dispatched body by + * `@objectstack/plugin-hono-server`'s rule, media type by media type, and + * refuses what it cannot read with `400 INVALID_REQUEST` when a dispatched + * domain READS the body, and only then — the rule `plugin-hono-server` applies + * to `req.body` — answering it with the same status and body as that package. * - * The catch-all used to hand `dispatch()` `{}` for such a body, which the data - * domain read as an empty query (every row) or an empty update (a `200` for a - * write that changed nothing). + * The catch-all used to hand `dispatch()` `{}` for a declared JSON body that + * did not parse (#23914), and, until #24042, it parsed every other body as + * JSON whatever it declared: a JSON body sent as `text/plain` was honoured, + * and any other body that did not declare JSON, a form included, was `{}`. + * The data domain read `{}` as an empty query (every row) or an empty update + * (a `200` for a write that changed nothing). * * The dispatcher is a fixture (this package's config aliases * `@objectstack/runtime` to a stub), standing in for the two kinds of domain: @@ -22,6 +26,11 @@ * `@objectstack/plugin-hono-server` resolves to its source here * (`vitest.config.ts`), so the parity case drives that package's real request * path, not a copy of it. + * + * In-process `Request`s carry no `Content-Length` of their own, and both doors + * judge whether a body is present by its framing headers (RFC 9112 §6.3), so + * a case that stands for a client's body sends `Content-Length` the way a + * client does. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; @@ -74,14 +83,38 @@ vi.mock('@objectstack/runtime', () => ({ import { createHonoApp } from './index'; const PREFIX = '/api/v1'; +const QUERY = '/data/showcase_account/query'; const kernel = { name: 'test-kernel' } as any; -const send = (path: string, method: string, contentType: string | undefined, body: string) => - createHonoApp({ kernel, prefix: PREFIX, cors: false }).request(`http://hosted.test${PREFIX}${path}`, { - method, - headers: contentType ? { 'Content-Type': contentType } : {}, - body, - }); +/** + * How a case frames its body. `length` sends `Content-Length` as a client + * does; `chunked` sends `Transfer-Encoding` instead. `bytes` sends the body as + * a `Uint8Array`: undici stamps `text/plain;charset=UTF-8` on any STRING body + * that declares no type, so a request with no `Content-Type` at all can only + * be built from bytes. + */ +interface Framing { length?: boolean; chunked?: boolean; bytes?: boolean } + +function requestInit(method: string, contentType: string | undefined, body: string | undefined, framing: Framing = {}): RequestInit { + const headers: Record = {}; + if (contentType) headers['content-type'] = contentType; + if (framing.length && body !== undefined) headers['content-length'] = String(Buffer.byteLength(body)); + if (framing.chunked) headers['transfer-encoding'] = 'chunked'; + const payload = framing.bytes && body !== undefined ? new TextEncoder().encode(body) : body; + return { method, headers, body: payload }; +} + +/** The hosted door: `createHonoApp`'s catch-all, in front of the fixture dispatcher. */ +const send = (path: string, method: string, contentType: string | undefined, body: string | undefined, framing?: Framing) => + createHonoApp({ kernel, prefix: PREFIX, cors: false }) + .request(`http://hosted.test${PREFIX}${path}`, requestInit(method, contentType, body, framing)); + +/** The self-hosted door: a `plugin-hono-server` route that reads `req.body` and answers what it read. */ +function sendDirect(method: string, contentType: string | undefined, body: string | undefined, framing?: Framing) { + const server = new HonoHttpServer(0); + server.post(`${PREFIX}/data/:object/query`, (req, res) => { res.json({ success: true, data: req.body }); }); + return server.getRawApp().fetch(new Request(`http://direct.test${PREFIX}${QUERY}`, requestInit(method, contentType, body, framing))); +} /** * The declared refusal, read whole. Structural rather than `ApiErrorSchema` @@ -89,14 +122,14 @@ const send = (path: string, method: string, contentType: string | undefined, bod * holds this body equal to `@objectstack/plugin-hono-server`'s, whose own suite * (`request-body-refusal.test.ts`) parses it against the declared envelope. */ -async function expectRefused(res: Response, body: string) { +async function expectRefused(res: Response, body: string, names: RegExp = /parsed as JSON/) { const json = await res.json() as any; expect(res.status, JSON.stringify(json)).toBe(400); expect(json).toEqual({ success: false, - error: { code: 'INVALID_REQUEST', message: expect.stringMatching(/parsed as JSON/) }, + error: { code: 'INVALID_REQUEST', message: expect.stringMatching(names) }, }); - expect(json.error.message).not.toContain(body.trim()); + if (body.trim() !== '') expect(json.error.message).not.toContain(body.trim()); return json; } @@ -108,8 +141,8 @@ beforeEach(() => { describe('[#23914] a domain that READS the body refuses the card\'s bodies, with this adapter\'s answer', () => { it.each([ - ['step 2 — a truncated query', 'POST', '/data/showcase_account/query', '{"where": {"status": "churned"}'], - ['step 3 — `{ not json`', 'POST', '/data/showcase_account/query', '{ not json'], + ['step 2 — a truncated query', 'POST', QUERY, '{"where": {"status": "churned"}'], + ['step 3 — `{ not json`', 'POST', QUERY, '{ not json'], ['step 5 — a truncated PATCH', 'PATCH', '/data/showcase_account/rec_1', '{"name": "HACKED"'], ['step 6 — a truncated create', 'POST', '/data/showcase_account', '{"name": "Door Truncated Create"'], ['a truncated PUT', 'PUT', '/meta/object/account', '{"name": "account"'], @@ -134,6 +167,38 @@ describe('[#23914] a domain that READS the body refuses the card\'s bodies, with }); }); +describe('[#24042] a body that declares neither JSON nor a form is refused by the domain that reads it', () => { + it.each([ + ['step 4 — a valid query body sent as text/plain', 'text/plain', '{"where": {"status": "churned"}}', /sent as text\/plain/], + ['text that does not declare JSON', 'text/plain', 'hello', /sent as text\/plain/], + ['bytes sent as application/octet-stream', 'application/octet-stream', 'raw bytes', /sent as application\/octet-stream/], + ])('%s: refused 400, though the domain answered its own 500', async (_label, contentType, body, names) => { + await expectRefused(await send(QUERY, 'POST', contentType, body, { length: true }), body, names); + expect(mockDispatcher.dispatch).toHaveBeenCalledTimes(1); + expect(seen).toEqual([]); + }); + + it('a body with no Content-Type at all is refused, not read as empty', async () => { + const body = '{"where": {"status": "churned"}}'; + await expectRefused(await send(QUERY, 'POST', undefined, body, { length: true, bytes: true }), body, /with no Content-Type/); + }); + + it('the /auth/* legacy fallback refuses it the same way', async () => { + await expectRefused( + await send('/auth/sign-in/email', 'POST', 'text/plain', '{"email": "a@b.c"}', { length: true }), + '{"email": "a@b.c"}', + /sent as text\/plain/, + ); + expect(mockDispatcher.handleAuth).toHaveBeenCalledTimes(1); + }); + + it('a form that does not parse is refused by its reader', async () => { + const body = '--nope\r\nnot a part'; + await expectRefused(await send(QUERY, 'POST', 'multipart/form-data; boundary=elsewhere', body), body, /form data/); + expect(seen).toEqual([]); + }); +}); + describe('[#23914] a domain that FORWARDS the request answers it itself, exactly as before', () => { it('the forwarder reads every byte the client sent and answers its own refusal; this adapter adds nothing', async () => { const body = '{deliveryId:'; @@ -143,6 +208,14 @@ describe('[#23914] a domain that FORWARDS the request answers it itself, exactly expect(seen).toEqual([{ path: '/webhooks/redeliver', forwarded: body }]); }); + it('[#24042] so does a body that does not declare JSON: the forwarder\'s own answer, every byte read', async () => { + const body = '{deliveryId:'; + const res = await send('/webhooks/redeliver', 'POST', 'text/plain', body, { length: true }); + expect(res.status).toBe(400); + expect(await res.json()).toEqual({ success: false, error: { code: 'INVALID_REQUEST', message: 'Request body must be JSON.' } }); + expect(seen).toEqual([{ path: '/webhooks/redeliver', forwarded: body }]); + }); + it('a well-formed body reaches the forwarder untouched', async () => { const body = '{ "deliveryId" : "d_1" }'; expect((await send('/webhooks/redeliver', 'POST', 'application/json', body)).status).toBe(200); @@ -150,14 +223,13 @@ describe('[#23914] a domain that FORWARDS the request answers it itself, exactly }); }); -describe('[#23914] CONTROL — what was readable is handed on as before', () => { +describe('[#23914, #24042] CONTROL — what is readable is handed on', () => { it.each([ ['a valid JSON body', 'application/json', '{"where": {"status": "churned"}}', { where: { status: 'churned' } }], ['an empty JSON body (an empty query)', 'application/json', '', {}], ['a whitespace-only JSON body', 'application/json', ' \n', {}], - ['a form body', 'application/x-www-form-urlencoded', 'token=abc', {}], - ['a JSON body sent as text/plain (the content-type-blind read, unchanged)', 'text/plain', '{"a":1}', { a: 1 }], - ['text that does not declare JSON (unchanged)', 'text/plain', 'hello', {}], + ['[#24042] a form body, parsed as plugin-hono-server parses it', 'application/x-www-form-urlencoded', 'token=abc', { token: 'abc' }], + ['[#24042] a request that declares no body, whatever its media type', 'text/plain', undefined, {}], ])('%s', async (_label, contentType, body, expected) => { const res = await send('/data/task/query', 'POST', contentType, body); expect(res.status).toBe(200); @@ -165,22 +237,58 @@ describe('[#23914] CONTROL — what was readable is handed on as before', () => }); }); -describe('[#23914] both Hono doors answer one unparseable body alike', () => { - it.each([ - ['a truncated query', '{"where": {"status": "churned"}'], - ['`{ not json`', '{ not json'], - ])('%s: the same status and the same body', async (_label, body) => { - const hosted = await send('/data/showcase_account/query', 'POST', 'application/json', body); - - const server = new HonoHttpServer(0); - server.post(`${PREFIX}/data/:object/query`, (req, res) => { res.json({ query: req.body }); }); - const direct = await server.getRawApp().fetch(new Request(`http://direct.test${PREFIX}/data/showcase_account/query`, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body, - })); - - expect(direct.status).toBe(hosted.status); - expect(await direct.json()).toEqual(await expectRefused(hosted, body)); +/** + * [#24042] One table, both doors. Each row is sent to the hosted door (the + * catch-all, in front of the reading domain) and to the self-hosted door (a + * `plugin-hono-server` route that reads `req.body`), and the two answers must + * be equal: the same status, and the same body — for a refusal, the same + * `code` and the same `message`; for a body that is read, the same value. + * + * [label, Content-Type, body, framing, the expected status] + */ +const PARITY: Array<[string, string | undefined, string | undefined, Framing, number]> = [ + // Declared JSON. + ['a valid JSON body', 'application/json', '{"where": {"status": "churned"}}', { length: true }, 200], + ['a truncated query (step 2)', 'application/json', '{"where": {"status": "churned"}', { length: true }, 400], + ['`{ not json` (step 3)', 'application/json', '{ not json', {}, 400], + ['declared JSON in another spelling', 'Application/JSON; charset=utf-8', '{ not json', { length: true }, 400], + ['a blank JSON body', 'application/json', '', { length: true }, 200], + ['a whitespace-only JSON body', 'application/json', ' \n', { length: true }, 200], + // A form. + ['a urlencoded form', 'application/x-www-form-urlencoded', 'token=abc&token=def&x=1', { length: true }, 200], + ['a malformed urlencoded form (read leniently by the parser on both doors)', 'application/x-www-form-urlencoded', 'a=%zz&b=%&=&&{"a":1}', { length: true }, 200], + ['a multipart form', 'multipart/form-data; boundary=b', '--b\r\nContent-Disposition: form-data; name="a"\r\n\r\n1\r\n--b--\r\n', { length: true }, 200], + ['a multipart form that does not parse', 'multipart/form-data; boundary=elsewhere', '--nope\r\nnot a part', { length: true }, 400], + // Any other media type, or none, with a body. + ['step 4 — `{"a":1}` sent as text/plain', 'text/plain', '{"a":1}', { length: true }, 400], + ['step 4 — the query body sent as text/plain;charset=UTF-8', 'text/plain;charset=UTF-8', '{"where": {"status": "churned"}}', { length: true }, 400], + ['plain text', 'text/plain', 'hello', { length: true }, 400], + ['a body with no Content-Type', undefined, '{"a":1}', { length: true, bytes: true }, 400], + ['application/octet-stream', 'application/octet-stream', 'raw bytes', { length: true }, 400], + ['a +json suffix type', 'application/merge-patch+json', '{"a":1}', { length: true }, 400], + ['a media type that is not a token pair (named as unrecognised)', 'text/plain extra', 'hello', { length: true }, 400], + ['a body framed by Transfer-Encoding', 'text/plain', '{"a":1}', { chunked: true }, 400], + // No body declared. + ['a text/plain request with no body', 'text/plain', undefined, {}, 200], + ['a text/plain body framed as zero-length', 'text/plain', '', { length: true }, 200], + ['an in-process text/plain body with no framing headers (read as no body on both doors)', 'text/plain', 'hello', {}, 200], +]; + +describe('[#23914, #24042] both Hono doors answer one body alike, media type by media type', () => { + it.each(PARITY)('%s: the same status and the same body', async (_label, contentType, body, framing, status) => { + const hosted = await send(QUERY, 'POST', contentType, body, framing); + const direct = await sendDirect('POST', contentType, body, framing); + + const hostedJson = await hosted.json() as any; + const directJson = await direct.json() as any; + expect(hosted.status, JSON.stringify(hostedJson)).toBe(status); + expect(direct.status, JSON.stringify(directJson)).toBe(status); + expect(hostedJson).toEqual(directJson); + if (status === 400) { + expect(hostedJson).toEqual({ success: false, error: { code: 'INVALID_REQUEST', message: expect.any(String) } }); + if (body && body.trim() !== '') expect(hostedJson.error.message).not.toContain(body.trim()); + } else { + expect(hostedJson.success).toBe(true); + } }); }); diff --git a/packages/adapters/hono/src/hono-catch-all-raw-request.test.ts b/packages/adapters/hono/src/hono-catch-all-raw-request.test.ts index 5331892d0bc..42c0de12bf9 100644 --- a/packages/adapters/hono/src/hono-catch-all-raw-request.test.ts +++ b/packages/adapters/hono/src/hono-catch-all-raw-request.test.ts @@ -20,10 +20,13 @@ * * 1. PRESERVATION — the `body` argument is the value the old line produced, for * every input: each case is checked against a reference Hono app that runs - * that exact line, and against the literal value written out. One input - * left this table on purpose (#23914): a declared `application/json` body - * that does not parse is handed on as a body whose READ is refused with a - * 400 (`hono-body-refusal.test.ts`); the request beside it still arrives + * that exact line, and against the literal value written out. Inputs left + * this table on purpose: a declared `application/json` body that does not + * parse (#23914), and, since the body is read by `plugin-hono-server`'s + * rule (#24042), a body that declares neither JSON nor a form (handed on + * as a body whose READ is refused with a 400) and a form (handed on + * parsed). Their answers are pinned in `hono-body-refusal.test.ts`, and + * the describes below pin that the request beside them still arrives * unread, every byte readable. * 2. THE FIX — at dispatch the request is unread (`bodyUsed === false`) and * reads back the exact bytes the client sent, JSON included. @@ -98,15 +101,14 @@ const CASES: ReadonlyArray { + const post = (contentType: string | undefined, text: string, bytes = false) => + createHonoApp({ kernel, prefix: PREFIX, cors: false }).request(`http://hosted.test${PATH}`, { + method: 'POST', + headers: { + ...(contentType ? { 'Content-Type': contentType } : {}), + 'Content-Length': String(Buffer.byteLength(text)), + }, + body: bytes ? new TextEncoder().encode(text) : text, + }); + + /** What reading the handed-on body throws; the fixture domain itself never reads it. */ + function readRefusal(body: unknown): unknown { + try { + JSON.stringify(body); + } catch (err) { + return err; + } + return undefined; + } + + it.each([ + ['JSON sent as text/plain', 'text/plain', '{"a":1}', /sent as text\/plain/], + ['invalid JSON that does not declare JSON', 'text/plain', '{ not json', /sent as text\/plain/], + ['plain text', 'text/plain', 'hello', /sent as text\/plain/], + ])('%s: handed on as a body whose read is refused 400, and every byte still readable on the request', async (_label, contentType, text, names) => { + const res = await post(contentType, text); + // The fixture domain records the body without reading it, so nothing + // refuses: the answer is the domain's. + expect(res.status).toBe(200); + expect(mockDispatcher.dispatch).toHaveBeenCalledTimes(1); + const [{ body, bodyUsed, text: forwarded }] = seen; + expect(readRefusal(body)).toMatchObject({ status: 400, code: 'INVALID_REQUEST', message: expect.stringMatching(names) }); + expect(bodyUsed).toBe(false); + expect(forwarded).toBe(text); + }); + + it('a body with no Content-Type at all: the same, and every byte still readable', async () => { + const text = 'token=abc'; + expect((await post(undefined, text, true)).status).toBe(200); + const [{ body, bodyUsed, text: forwarded }] = seen; + expect(readRefusal(body)).toMatchObject({ status: 400, code: 'INVALID_REQUEST', message: expect.stringMatching(/with no Content-Type/) }); + expect(bodyUsed).toBe(false); + expect(forwarded).toBe(text); + }); + + it('a form body: handed on parsed (the last value of a repeated name, as Hono\'s parseBody answers), and every byte still readable', async () => { + const text = 'token=abc&token=def&x=1'; + expect((await post('application/x-www-form-urlencoded', text)).status).toBe(200); + expect(seen).toHaveLength(1); + expect(seen[0]!.body).toEqual({ token: 'def', x: '1' }); + expect(seen[0]!.bodyUsed).toBe(false); + expect(seen[0]!.text).toBe(text); + }); + + it('a form body something upstream already read through c.req: the parsed form still arrives, from Hono\'s cache', async () => { + const parent = new Hono(); + parent.use('*', async (c, next) => { await c.req.parseBody(); await next(); }); + parent.route('/', createHonoApp({ kernel, prefix: PREFIX, cors: false })); + + await parent.request(`http://hosted.test${PATH}`, { + method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body: 'token=abc', + }); + expect(seen).toHaveLength(1); + expect(seen[0]!.body).toEqual({ token: 'abc' }); + expect(seen[0]!.bodyUsed).toBe(true); + }); +}); diff --git a/packages/adapters/hono/src/index.ts b/packages/adapters/hono/src/index.ts index fb199e0ebef..a4b335ae969 100644 --- a/packages/adapters/hono/src/index.ts +++ b/packages/adapters/hono/src/index.ts @@ -3,6 +3,7 @@ import { Hono } from 'hono'; import type { Context } from 'hono'; import { cors } from 'hono/cors'; +import { parseBody } from 'hono/utils/body'; import { type ObjectKernel, HttpDispatcher, @@ -282,8 +283,9 @@ export function objectStackMiddleware(kernel: ObjectKernel) { } /** - * The JSON body this adapter hands the dispatcher, read WITHOUT consuming the - * request (#22576). + * The body this adapter hands the dispatcher: read by its declared media type, + * by `@objectstack/plugin-hono-server`'s rule, or REFUSED when read (#24042); + * never invented, and never read off the request it forwards (#22576). * * ## Why the raw request must stay readable * @@ -299,86 +301,108 @@ export function objectStackMiddleware(kernel: ObjectKernel) { * verify an HMAC over the sender's exact JSON bytes — and a spaced JSON body * re-serialised from the parsed value is not those bytes. * - * So the parse reads a CLONE: `clone()` tees the body, `c.req.raw` keeps the - * other branch unread, and a domain that forwards the request gets every byte - * the client sent, for every `Content-Type`, JSON included. + * So whatever is parsed is parsed from a CLONE: `clone()` tees the body, + * `c.req.raw` keeps the other branch unread, and a domain that forwards the + * request gets every byte the client sent, for every `Content-Type`. One case + * cannot be read from a clone: a body something upstream already consumed + * through `c.req` — Hono's body cache then holds the only copy, and it is read + * from that cache (`c.req.text()`, `c.req.parseBody()`), as + * `plugin-hono-server` reads it. * - * ## What the domains see is unchanged, deliberately + * ⚠️ The cost is the tee: until the request is released, the unread branch + * holds the body's bytes, so a JSON or form body is buffered twice instead of + * once for the life of the request. These are API bodies the old path already + * buffered whole; nothing here streams. A body of any other media type is not + * read here at all. * - * Hono's `c.req.json()` is `text()` then `JSON.parse`, with any failure mapped - * to `{}` by the `.catch` this adapter put on it — so an empty body, a form - * body and invalid JSON all became `{}`, and any JSON value (an array, `null`, - * a number) came through as parsed. The clone is parsed by the same two steps - * and every failure is the same `{}`, so the `body` argument is the value it - * was for every input. One case cannot be read from a clone: a body something - * upstream already consumed through `c.req` — Hono's body cache then holds the - * only copy, and `c.req.json()` is asked exactly as before. + * ## What a reading answers: `plugin-hono-server`'s rule (#24042) * - * ⚠️ The cost is the tee: until the request is released, the unread branch - * holds the body's bytes, so a body is buffered twice instead of once for the - * life of the request. These are API bodies the old path already buffered - * whole; nothing here streams. - * - * ## A declared JSON body that does not parse is refused BY ITS READER (#23914) - * - * …and is no longer `{}`. That `{}` reached the data domain as an empty query, - * so a truncated `POST /data/:object/query` answered `200` with every row the - * caller could read, and a truncated `PATCH` answered `200` for an update that - * changed nothing. - * - * One rule on both Hono transports: the code that READS the parsed body - * refuses it, nothing earlier. `@objectstack/plugin-hono-server` does that - * with its `req.body` accessor; here the body is a `dispatch()` argument, so - * for a body whose `Content-Type` declares `application/json` (matched - * case-insensitively), is not blank, and does not parse, the value handed on - * is {@link refusedOnRead}'s: any read of it — a property, `in`, its keys, a - * spread, `JSON.stringify` — throws {@link RequestBodyRefusal} (a declared - * `400` with the registered code `INVALID_REQUEST`) and records that it was - * read. Both arms that call this then answer that refusal, through - * `thrownJson` as the ADR-0112 envelope, whatever the domain made of the throw - * — the same override `plugin-hono-server` applies — and only then. + * One rule on both Hono transports. It is `readRequestBody` in + * `@objectstack/plugin-hono-server` (`src/request-body.ts`), module-private + * there and mirrored here, media type by media type: + * + * - **Declared JSON** (`Content-Type` includes `application/json`, matched + * case-insensitively): read as text and parsed. Empty or whitespace-only is + * `{}` (the REST data doors document an empty body as an empty query). A + * body that does not parse, or cannot be read, is refused (#23914). + * - **A form** (`multipart/form-data`, `application/x-www-form-urlencoded`): + * parsed by Hono's own `parseBody`, the function `c.req.parseBody()` runs on + * the other door. A form that does not parse is refused. + * - **Any other media type, or none**: nothing is read. When the request + * declares a body by its framing headers ({@link declaresBody}), it is + * refused; when it declares none, it is `{}`. + * + * The content-type-blind parse this replaces read a JSON body sent as + * `text/plain` as JSON, and handed `{}` for every other body that did not + * declare JSON, a form included. `{}` reached the data domain as an empty + * query, so a garbled body that did not declare JSON read the whole table on + * this door, while `plugin-hono-server` refused it. The wire contract names + * `Content-Type: application/json` as required + * (`content/docs/api/wire-format.mdx`); parsing `text/plain` as JSON is a + * second dialect, so it is refused, not parsed. + * + * ## A body that cannot be read is refused BY ITS READER (#23914) + * + * …and is never `{}`. The code that READS the parsed body refuses it, nothing + * earlier. `@objectstack/plugin-hono-server` does that with its `req.body` + * accessor; here the body is a `dispatch()` argument, so the value handed on + * for every refusal above is {@link refusedOnRead}'s: any read of it — a + * property, `in`, its keys, a spread, `JSON.stringify` — throws + * {@link RequestBodyRefusal} (a declared `400` with the registered code + * `INVALID_REQUEST`) and records that it was read. Both arms that call this + * then answer that refusal, through `thrownJson` as the ADR-0112 envelope, + * whatever the domain made of the throw — the same override + * `plugin-hono-server` applies — and only then. * * Why on read and not before `dispatch()`. A domain that forwards the request * reads its own bytes from it and never reads `body`: `/webhooks/redeliver` * and the inbound automation hooks answer a body that is not JSON with their * own refusal, after their own sign-in or signature check, and the - * `/approvals/act` form `POST` reads a form. Refusing before `dispatch()` - * pre-empted them at this door only (measured: `/webhooks/redeliver` then - * answered this adapter's message where the self-hosted mount answers the - * member's, and an anonymous caller got `400` where the mount says `401`). - * `dispatch()` itself never reads `body` (its gates read headers and the - * path), so the gates still run first, as they do ahead of a route handler on - * `plugin-hono-server`. No list of routes is kept here: whether a domain reads - * the body is answered by the domain reading it. - * - * `@objectstack/plugin-hono-server` answers the same body with the same `code` - * and `message` (`hono-body-refusal.test.ts` drives both doors). - * - * Every other input is read exactly as before, deliberately: - * - an empty or whitespace-only body is `{}` (an empty query); - * - a body that does not declare JSON keeps the content-type-blind parse and - * its `{}` on failure; - * - the request handed on beside it is unread in every case, so a domain that - * forwards it reads every byte the client sent, a malformed body included. + * `/approvals/act` form `POST` reads its form from the request. Refusing + * before `dispatch()` pre-empted them at this door only (measured: + * `/webhooks/redeliver` then answered this adapter's message where the + * self-hosted mount answers the member's, and an anonymous caller got `400` + * where the mount says `401`). `dispatch()` itself never reads `body` (its + * gates read headers and the path), so the gates still run first, as they do + * ahead of a route handler on `plugin-hono-server`. No list of routes is kept + * here: whether a domain reads the body is answered by the domain reading it. + * + * `@objectstack/plugin-hono-server` answers the same body with the same + * `status`, `code` and `message` (`hono-body-refusal.test.ts` drives both + * doors over one table). */ async function readDispatchBody(c: Context): Promise { - const declaresJson = (c.req.header('content-type') ?? '').toLowerCase().includes('application/json'); + const lowered = (c.req.header('content-type') ?? '').toLowerCase(); const raw = c.req.raw; - let text: string; - try { - // A body something upstream already read through `c.req` is in Hono's - // body cache, which is the only copy left; `text()` reads that cache. - text = raw.bodyUsed ? await c.req.text() : await raw.clone().text(); - } catch { - return declaresJson ? refusedOnRead(new RequestBodyRefusal(UNREADABLE_BODY_MESSAGE)) : readable({}); + + if (lowered.includes('application/json')) { + let text: string; + try { + text = raw.bodyUsed ? await c.req.text() : await raw.clone().text(); + } catch { + return refusedOnRead(new RequestBodyRefusal(UNREADABLE_BODY_MESSAGE)); + } + if (text.trim() === '') return readable({}); + try { + return readable(JSON.parse(text)); + } catch { + return refusedOnRead(new RequestBodyRefusal(MALFORMED_JSON_BODY_MESSAGE)); + } } - try { - return readable(JSON.parse(text)); - } catch { - return declaresJson && text.trim() !== '' - ? refusedOnRead(new RequestBodyRefusal(MALFORMED_JSON_BODY_MESSAGE)) - : readable({}); + + const mediaType = lowered.split(';')[0]!.trim(); + if (mediaType === 'multipart/form-data' || mediaType === 'application/x-www-form-urlencoded') { + try { + return readable(raw.bodyUsed ? await c.req.parseBody() : await parseBody(raw.clone())); + } catch { + return refusedOnRead(new RequestBodyRefusal(malformedFormMessage(mediaType))); + } } + + // Not read: a domain that wants these bytes reads them off the request. + return declaresBody(c) + ? refusedOnRead(new RequestBodyRefusal(unreadMediaTypeMessage(mediaType))) + : readable({}); } /** What {@link readDispatchBody} hands an arm: the `body` argument, and whether a refused body was read. */ @@ -422,7 +446,8 @@ function refusedOnRead(refusal: RequestBodyRefusal): DispatchBody { * throw (`status` and the registered `INVALID_REQUEST`), so `thrownJson` * renders it through `declaredEnvelopeForThrow`. Module-private, like its twin * in `@objectstack/plugin-hono-server` (`src/request-body.ts`); the messages - * are the twin's, word for word, and a test holds the two doors' answers equal. + * below and {@link declaresBody} are the twin's, word for word, and a test + * holds the two doors' answers equal (`hono-body-refusal.test.ts`). */ class RequestBodyRefusal extends Error { readonly status = 400; @@ -434,11 +459,45 @@ class RequestBodyRefusal extends Error { } } +/** The refusal tail every message ends with: what the transport did NOT do. */ +const NOT_EMPTY = 'It was refused rather than read as an empty body'; + const MALFORMED_JSON_BODY_MESSAGE = - 'The request body could not be parsed as JSON (it was sent as application/json). It was refused rather than ' - + 'read as an empty body: send a well-formed JSON document, or no body at all.'; + `The request body could not be parsed as JSON (it was sent as application/json). ${NOT_EMPTY}: ` + + 'send a well-formed JSON document, or no body at all.'; -const UNREADABLE_BODY_MESSAGE = 'The request body could not be read. It was refused rather than read as an empty body.'; +const UNREADABLE_BODY_MESSAGE = `The request body could not be read. ${NOT_EMPTY}.`; + +/** [#24042] A media type as the client spelled it, echoed only when it is a plain token pair. */ +function namedMediaType(mediaType: string): string { + if (mediaType === '') return 'with no Content-Type'; + return /^[a-z0-9][a-z0-9!#$&^_.+-]{0,63}\/[a-z0-9][a-z0-9!#$&^_.+-]{0,63}$/.test(mediaType) + ? `as ${mediaType}` + : 'with an unrecognised Content-Type'; +} + +function malformedFormMessage(mediaType: string): string { + return `The request body could not be parsed as form data (it was sent ${namedMediaType(mediaType)}). ${NOT_EMPTY}.`; +} + +function unreadMediaTypeMessage(mediaType: string): string { + return `The request body was sent ${namedMediaType(mediaType)}, which this server does not read into a ` + + `request body. ${NOT_EMPTY}: send it as application/json.`; +} + +/** + * [#24042] Whether the request carries a body, by RFC 9112 §6.3: a `Content-Length` + * that is not `0`, or a `Transfer-Encoding`. Reading the bytes to find out would + * buffer a body this adapter never reads, and `Request.body` cannot answer: + * `@hono/node-server` hands every non-GET/HEAD request a stream, an empty one + * included. An in-process `Request` built without those headers is therefore + * read as having no body — on both doors. + */ +function declaresBody(c: Context): boolean { + const length = c.req.header('content-length'); + if (length !== undefined && length.trim() !== '') return Number(length) !== 0; + return c.req.header('transfer-encoding') !== undefined; +} /** * The query the catch-all hands `dispatch()`: a REPEATED name as an array, a