Repository navigation
Commit acacab0
fix(qa): the reference IHttpServer adapter refuses, parses or reads req.body as the IHttpRequest.body contract says (#24242)
Fixes #24083
Clause-②: no
## What changed
`packages/qa/http-conformance/src/adapter.ts`: the reference
`NodeHttpServer` now decides `req.body` the way the standard transport
decides it (`readRequestBody` in `plugin-hono-server`'s
`src/request-body.ts`). Per the seat's ruling (MATCH) the decision is
mirrored inside `adapter.ts` and not imported. The package keeps
`@objectstack/core` as its only runtime dependency, and no export was
added anywhere.
| Body sent | Reference before (`origin/main` `2bcd406a2`) | Both
adapters after |
|---|---|---|
| declared JSON that does not parse (`{ not json`) | 200, `req.body` is
`{}` | 400 `INVALID_REQUEST`, thrown from the read |
| non-empty `text/plain` | 200, `req.body` is the string | 400
`INVALID_REQUEST`, thrown from the read |
| `Text/Plain; charset=utf-8` (case) | 200, `{}` | 400 `INVALID_REQUEST`
|
| empty `text/plain` | 200, `""` | 200, `{}` |
| multipart with fields | 200, `{}` | 200, the fields: last value wins,
a `[]` key collects an array, a file part is a `File` |
| malformed or empty multipart | 200, `{}` | 400 `INVALID_REQUEST` |
| urlencoded `a=1&a=2&b[]=3&b[]=4` | `{"a":"2","b[]":"4"}` |
`{"a":"2","b[]":["3","4"]}` |
| handler catches the refusal, answers 500 | (no refusal) | 400
`INVALID_REQUEST`: the refusal replaces the handler's answer |
| a fallback (`setFallbackHandler`) reads a refused body, then throws,
answers its own, or writes nothing | 500 `Fallback handler failed`, its
own answer, or the 404/405 | 400 `INVALID_REQUEST` on all three (patch
round 1) |
| `rawBody()` on a body `req.body` refuses | the bytes | the bytes
(unchanged) |
I measured all 17 request shapes with one probe that drives both
adapters over a real socket, and, in patch round 1, 12 fallback shapes
with a second probe. After the change, every shape answers the same
status and the same body bytes on both. The one difference is the
`charset=utf-8` parameter on the reference's JSON `Content-Type`. Every
reference JSON answer already carried it before this change.
How it works:
- **The decision.** One module-private `readRequestBody` copies the
standard transport's arms and its four refusal messages word for word.
- Declared JSON (matched case-insensitively): empty or whitespace is
`{}`, anything else that does not parse is refused.
- A form, `multipart/form-data` or `application/x-www-form-urlencoded`,
is parsed by Node's own Fetch `Response.formData()`. This is the parser
hono's `bufferToFormData` calls. The field shape follows hono 4.13.12's
`convertFormDataToBodyData` with `all: false, dot: false`, and its null
prototype too.
- For any other media type nothing is read. The body is refused on read
when the framing headers declare one (RFC 9112 §6.3), and is `{}`
otherwise.
- GET and HEAD read no bytes from the socket, as `@hono/node-server`
hands them a request with no body.
- **`req.body` is an accessor** with a setter, as on the standard
transport. A route that reads only `rawBody()` never meets the refusal.
`rawBody()` now caches the read as one promise, so a body the decision
already consumed hands back the same bytes, or the same read failure.
- **The answer side** (mechanism assumption 1, measured below). On a
matched route and in the fallback alike, once a read of `req.body` meets
the refusal, anything the handler writes after it is dropped.
`writeBodyRefusal` then answers `400
{"success":false,"error":{"code":"INVALID_REQUEST","message":...}}`, the
two keys the standard transport's `declaredEnvelopeForThrow` renders for
this throw. In the fallback branch (patch round 1, seat ruling
`6244250024`) it answers on every arm: the fallback threw, caught and
answered, or caught and wrote nothing. This is what the standard
transport's fallback answers, because it runs through the same
`runHandler` as its routes. Untouched: the generic catch arm, which
reads `statusCode` only and answers an `{error}` body
(`adapter.test.ts`'s `maps thrown errors with statusCode` pins it), and
the fallback's `500 Fallback handler failed` for a throw that read no
refused body (`fallback-seam.conformance.test.ts` G5-a pins it).
Guarantee 4's docblock now names the one exception: a fallback that read
a refused body.
New `packages/qa/http-conformance/src/request-body.conformance.test.ts`.
Its route handlers read `req.body` and run on BOTH adapters over a real
socket. Per adapter, it covers:
- malformed declared JSON;
- non-empty `text/plain`;
- multipart fields: single, repeated, `[]`, a file part;
- a handler that catches the refusal and writes its own answer;
- `rawBody()` of a refused body;
- the controls: no body, an empty JSON body and an empty `text/plain`
body are each `{}`, and valid JSON is the document.
- the fallback (patch round 1): a fallback that reads a refused body and
then throws, catches and answers, or catches and writes nothing is
answered with the refusal. Its controls: a fallback that never reads the
body and writes nothing leaves the 404, and the 405 + `Allow`; one that
reads a valid JSON body gets the document.
A parity block then sends each of 16 rows to both adapters and asserts
the same status and the same bytes, refusal messages included. A change
to either side's wording therefore reds here.
`packages/qa/http-conformance/README.md` (patch round 1): the one line
that listed multipart parsing as deliberately absent now says what the
adapter does. JSON and forms (multipart included) are parsed into
`req.body`, and every other media type stays raw behind the lazy
`req.rawBody()`.
## PM mechanism assumptions, measured
1. **Partly falsified.**
- Held: the standard transport refuses from the `body` read, not before
the handler, and a route that reads only `rawBody()` is unaffected.
- Did not hold: its answer does not take "the same declared-error path
every thrown declared error takes". `runHandler` has a dedicated
`refusedBodyRead` arm that answers the refusal whatever the handler made
of the throw, a caught throw included (probe: a handler that caught it
and wrote 500 still answered 400). Only the renderer,
`declaredEnvelopeForThrow`, is shared.
- The reference did not render a declared throw at all. A route throwing
`{ status: 400, code: 'INVALID_REQUEST' }` answered `500
{"error":"declared refusal"}`. So the answer-side change above was
required. It is the smallest change that reproduces the replacement, and
it does not touch the generic catch arm.
2. **Held.** The standard transport parses both form types through
`parseBody()`. Node's built-in `Response.formData()` matches every field
shape the case reaches without a dependency: a single value, a repeated
key (last wins), a `[]` key (array) and a file part (`File`).
3. **Held.** `rawBody()` reads the bytes of every body, refused ones
included. The suite pins it for malformed JSON and `text/plain`.
Multipart `rawBody()` is the held-off hono cache question and is not
asserted.
4. **Held.** No existing assert pinned an old answer, so
`adapter.test.ts` is unchanged. `fallback-seam.conformance.test.ts` is
unchanged too: its only 500 assert, G5-a, is a fallback that throws its
own error on a body-less GET and never reads a refused body. These
suites drive the reference and read `body`, and all pass on `6544c545a`:
- `adapter.test.ts` (16)
- `fallback-seam.conformance.test.ts` (18)
- `conformance.integration.test.ts` (30)
- `query-multiplicity.conformance.test.ts` (8)
- `response-observation.conformance.test.ts` (14)
## Ablation (once, the card's acceptance 3)
`node scripts/ablation-replace.mjs` (WRAP mode, with an `EXIT INT TERM`
trap) restored the old arm `let body: any = {}; try { body = text ?
JSON.parse(text) : {}; } catch { body = {}; }` in place of the
malformed-JSON refusal. No build is involved: the suite imports
`./adapter.js` from source.
- The mutation landed on disk: anchor 1 to 0, old arm 0 to 1, blob
`8204c4fa1a35` to `9b952dea3e0e`.
- **Red:** `pnpm --filter @objectstack/http-conformance exec vitest run
--maxWorkers=2 src/request-body.conformance.test.ts`, verify-lock
`VERDICT command-exit 1`, `Tests 2 failed | 22 passed (24)`. The two
failures:
- `... on the 'node' adapter > a malformed declared-JSON body is refused
400 INVALID_REQUEST at the read, never read as {}`
- the parity row `/echo · malformedJson`
- Every `hono` leg stayed green. The parity row is cross-adapter by
construction, so it reds whenever the reference drifts.
- **Restore** proved: blob equals HEAD (`8204c4fa1a35`), and `git diff
HEAD` is empty.
- **Green:** same command on `9e5b8b1e2`, `VERDICT command-exit 0`,
`Tests 24 passed (24)`.
Patch round 1, once: `node scripts/ablation-replace.mjs --delete` (WRAP
mode, with an `EXIT INT TERM` trap) removed the fallback branch's one
refusal-answer line, `if (refusedBodyRead) return
this.writeBodyRefusal(nodeRes, refusedBodyRead);`.
- The mutation landed on disk: line count 1 to 0, blob `d3cdbc3cfd7a` to
`54509553a9f2`.
- **Red:** the same vitest command, `VERDICT command-exit 1`, `Tests 6
failed | 34 passed (40)`. The six failures:
- the three `node` fallback legs: the refused-read-then-throw leg
answered `500 Fallback handler failed`, and the caught-and-answered and
caught-and-silent legs answered `404 ENDPOINT_NOT_FOUND`;
- their three parity rows: `/fallback/throw`,
`/fallback/caught-answers`, `/fallback/caught-silent`.
- Every `hono` leg stayed green, and so did both fallback controls.
- **Restore** proved: blob equals HEAD (`d3cdbc3cfd7a`), and `git diff
HEAD` is empty.
- **Green:** the whole suite on `6544c545a`, `Tests 190 passed (190)`,
with `request-body.conformance.test.ts` at 40 of 40.
## Tests (final head `6544c545a`, after merging `origin/main`
`7be3cc728`)
- First rebuilt the closure under the verify lock: `pnpm turbo run build
--filter='@objectstack/http-conformance^...' --concurrency=2`, `VERDICT
command-exit 0`, 36/36 tasks.
- `pnpm --filter @objectstack/http-conformance exec vitest run
--reporter=verbose`: exit 0, `Test Files 15 passed (15)`, `Tests 190
passed (190)`.
- `pnpm --filter @objectstack/http-conformance typecheck`: exit 0. This
and the line above ran joined by `&&` under one `VERDICT command-exit
0`.
- `tsc --noEmit` passes, and so does `check:test-typecheck`: 3 files /
27 errors / 10 signatures held, unchanged.
- The new test file carries no ledger entry, so it compiles with zero
errors.
- `test:repo`, each exit captured on its own, all 0 (re-run in patch
round 1, spec included, since its repo tests walk `packages/**`):
| Package | Files | Tests |
|---|---|---|
| `@objectstack/core` | 6 | 86 |
| `@objectstack/objectql` | 1 | 5 |
| `@objectstack/rest` | 5 | 262 passed, 14 skipped |
| `@objectstack/runtime` | 3 | 808 |
| `@objectstack/types` | 1 | 11 |
| `@objectstack/spec` | 67 | 1115 |
## Gates (final head `6544c545a`)
- `pnpm exec eslint --no-inline-config` on both changed `.ts` files:
exit 0, no output.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived the same 52 commands as round 0. 51
exited 0.
- `pnpm check:dual-build-cjs-loads` exited 3 (`PREREQUISITE NOT MET`):
it reads every package's `dist/`, and only this package's closure is
built locally. NOT MEASURED locally; CI's `Lint & Repo Gates` runs it
and is green on `6544c545a`.
- `dispatch-gates --ran`: `52 derived famil(ies) accounted for — 51 run,
1 NOT-MEASURED (1 DERIVED from a recorded exit 3)`.
- Clause-② judged from this diff:
- `scripts/pm/clause2-line.mjs` has no CLI. Run bare it prints nothing
and exits 0. Its `readClause2Line` export reads this body's line as
`{"kind":"declared","value":"no","arm":null}`.
- `node scripts/check-changeset-no-major.mjs --base origin/main --event`
(this body) on the final diff: exit 0, its `LEVEL AXIS` line for a
declared clause-② `no`, with no direction arm. No narrowing face was
read.
## Changeset
None. `@objectstack/http-conformance` is `"private": true` and publishes
nothing, which is the repo's private-package fast lane for
`skip-changeset`. The package's `CHANGELOG.md` has only ever carried
dependency bumps, and the precedent test-only change `6a53564b9` shipped
without one. The `skip-changeset` label is applied after this PR is
created.
## Acceptance notes
- **README line (fixed in patch round 1).** The README line that listed
multipart parsing as deliberately absent is corrected in this PR, per
seat ruling `6244250024`.
- **Fallback seam (fixed in patch round 1).** A fallback that reads a
refused body is now answered with the refusal on the reference too, on
every arm, as on the standard transport. Seat ruling `6244250024` lifted
the hold-off for the answer path only.
- **Fallback contract wording (spec, held off).** In
`packages/spec/src/contracts/http-server.ts`, the `setFallbackHandler`
sentence "A handler that writes no response leaves the adapter's
standard unmatched-request answer in place" names no exception. Both
adapters now answer a fallback that read a refused body and wrote
nothing with `400 INVALID_REQUEST`, as the `IHttpRequest.body` paragraph
says the standard transport does. The wording is the spec lane's;
nothing here edits it.
- **Handler writes before it reads.** A handler that WROTE before it
read a refused body cannot be replaced on `node:http`: its bytes are
already on the wire. The standard transport buffers the answer and still
replaces it. The code documents this; no conformance case reaches it.
- **Middleware reads `req.body`.** `use()` middleware on the reference
receives the real request, so a middleware that reads `req.body` meets
the refusal. The standard transport hands middleware an empty request.
The middleware contract does not populate `body`, and no case reaches
it.
- **Content-Type charset.** The reference's JSON answers carry
`Content-Type: application/json; charset=utf-8`, and the standard
transport's carry `application/json`. This predates the change and
covers every reference JSON answer.
- **Hono multipart `rawBody()` cache.** The hold-off named by the card
is not touched or asserted here.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01BVm1pAXVXQ3PjC42j1kPFf)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 646303e commit acacab0
3 files changed
Lines changed: 574 additions & 32 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
44 | | - | |
| 43 | + | |
| 44 | + | |
45 | 45 | | |
46 | 46 | | |
47 | 47 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
98 | 98 | | |
99 | 99 | | |
100 | 100 | | |
101 | | - | |
102 | | - | |
103 | | - | |
104 | | - | |
105 | | - | |
106 | | - | |
107 | | - | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
108 | 257 | | |
109 | 258 | | |
110 | 259 | | |
| |||
211 | 360 | | |
212 | 361 | | |
213 | 362 | | |
214 | | - | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
215 | 366 | | |
216 | 367 | | |
217 | 368 | | |
| |||
285 | 436 | | |
286 | 437 | | |
287 | 438 | | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
288 | 464 | | |
289 | 465 | | |
290 | 466 | | |
| |||
373 | 549 | | |
374 | 550 | | |
375 | 551 | | |
376 | | - | |
377 | | - | |
378 | | - | |
379 | | - | |
380 | | - | |
381 | | - | |
382 | | - | |
383 | | - | |
384 | | - | |
385 | | - | |
386 | | - | |
387 | | - | |
388 | | - | |
389 | | - | |
390 | | - | |
391 | | - | |
392 | | - | |
393 | | - | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
394 | 561 | | |
395 | 562 | | |
396 | 563 | | |
| |||
400 | 567 | | |
401 | 568 | | |
402 | 569 | | |
403 | | - | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
404 | 586 | | |
405 | 587 | | |
406 | 588 | | |
407 | | - | |
| 589 | + | |
| 590 | + | |
408 | 591 | | |
409 | 592 | | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
410 | 602 | | |
411 | 603 | | |
412 | 604 | | |
413 | 605 | | |
414 | 606 | | |
415 | 607 | | |
416 | 608 | | |
417 | | - | |
| 609 | + | |
418 | 610 | | |
419 | 611 | | |
420 | 612 | | |
421 | 613 | | |
422 | | - | |
| 614 | + | |
423 | 615 | | |
424 | 616 | | |
425 | 617 | | |
| |||
432 | 624 | | |
433 | 625 | | |
434 | 626 | | |
| 627 | + | |
435 | 628 | | |
436 | 629 | | |
437 | 630 | | |
438 | 631 | | |
| 632 | + | |
439 | 633 | | |
440 | 634 | | |
441 | 635 | | |
| |||
447 | 641 | | |
448 | 642 | | |
449 | 643 | | |
| 644 | + | |
450 | 645 | | |
451 | 646 | | |
452 | 647 | | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
453 | 656 | | |
454 | 657 | | |
455 | 658 | | |
| |||
481 | 684 | | |
482 | 685 | | |
483 | 686 | | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
484 | 691 | | |
485 | 692 | | |
486 | 693 | | |
| |||
493 | 700 | | |
494 | 701 | | |
495 | 702 | | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
496 | 707 | | |
497 | 708 | | |
498 | 709 | | |
| |||
0 commit comments