Skip to content

Commit 5ef104d

Browse files
committed
fix(rest)!: the remaining numeric query reads refuse what they cannot read, held by a census
History sinceSeq and audit limit read through their declarations; diff from/to, search perObject and approvals limit/offset read as whole numbers, all through readDeclaredQueryNumber. A census test classifies every numeric coercion in rest-server.ts so a new bare Number() over a query value reddens its PR. Export page and rollback toVersion are the ledgered exemptions, each pinned. The publish route comment names 404 NO_DRAFT instead of the retired [no_draft] opener. Claude-Session: https://claude.ai/code/session_01UYBdGBzWSrAMzpW8ah3GbP Co-authored-by: Claude <noreply@anthropic.com>
1 parent 54658aa commit 5ef104d

4 files changed

Lines changed: 661 additions & 47 deletions

File tree

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
---
2+
'@objectstack/rest': minor
3+
---
4+
5+
fix(rest): the remaining numeric query reads refuse a value they cannot read with `400 VALIDATION_FAILED`, instead of dropping it or handing on `NaN` and answering `200` (#20139)
6+
7+
Clause-②: no (narrowing)
8+
9+
**BREAKING**: shipped as `minor` under the launch-window convention
10+
(`check-changeset-no-major` refuses `major` until GA). The banner and the ADR-0087
11+
disposition below carry the breaking-ness, not the level.
12+
13+
Five published doors still read a numeric query parameter with a bare `Number()`.
14+
That coercion does not fail. It invents `NaN` or `0`, and the door dropped it or
15+
served it with a `200`:
16+
- `GET /meta/:type/:name/history?sinceSeq=abc` read the change log from the start;
17+
- `GET /meta/:type/:name/audit?limit=abc` served the producer's default 100 events;
18+
- `GET /meta/:type/:name/diff?from=abc` diffed a different pair of versions;
19+
- `GET /search?perObject=abc` removed the per-object cap;
20+
- `GET /approvals/requests?limit=abc` served the unpaged 500-row window instead of a page.
21+
22+
Each door now reads the parameter the way `?limit=` on import jobs, export, history and
23+
search already does:
24+
- **`GET /meta/:type/:name/history`** reads `sinceSeq` through
25+
`HistoryMetaItemRequestSchema.sinceSeq` (`z.number()`, so any finite number, as before).
26+
- **`GET /meta/:type/:name/audit`** reads `limit` through
27+
`AuditMetaItemRequestSchema.limit` (`z.number()`; the implementation's `[1, 500]`
28+
clamp is unchanged).
29+
- **`GET /meta/:type/:name/diff`** (`from` / `to`, and their `fromVersion` /
30+
`toVersion` spellings), **`GET /search`** (`perObject`) and
31+
**`GET /approvals/requests`** (`limit` / `offset`) declare no request schema. There the
32+
value must be a whole number. Each service's own range handling is unchanged.
33+
34+
A value the door cannot read answers `400` with the data surface's existing envelope,
35+
`{ error, code: 'VALIDATION_FAILED', fields }`. `fields[0].field` names the parameter as
36+
the caller spelled it, and `fields[0].code` is `invalid_type`. The service is never
37+
called. On `GET /approvals/requests` this is a `400`, not the route's
38+
`500 APPROVAL_REQUEST_LIST_FAILED`.
39+
40+
What changes, per door (every row answered `200` before):
41+
42+
| door | request | answered before | answers now |
43+
|:--|:--|:--|:--|
44+
| `GET /meta/:type/:name/history` | `?sinceSeq=abc`, `?sinceSeq=Infinity` | the change log from the start | `400`, `invalid_type` |
45+
| `GET /meta/:type/:name/history` | `?sinceSeq=` (empty) | `sinceSeq: 0` applied as a cursor | `400`, `invalid_type` |
46+
| `GET /meta/:type/:name/audit` | `?limit=abc`, `?limit=Infinity` | the default 100 events | `400`, `invalid_type` |
47+
| `GET /meta/:type/:name/audit` | `?limit=` (empty) | one event | `400`, `invalid_type` |
48+
| `GET /meta/:type/:name/diff` | `?from=abc`, `?from=Infinity` | the version before `to`, diffed instead | `400`, `invalid_type` |
49+
| `GET /meta/:type/:name/diff` | `?to=abc` | the current body, diffed instead | `400`, `invalid_type` |
50+
| `GET /meta/:type/:name/diff` | `?from=1.5`, `?to=2.5` | a diff against a version that cannot exist | `400`, `invalid_type` |
51+
| `GET /search` | `?perObject=abc` | no per-object cap | `400`, `invalid_type` |
52+
| `GET /search` | `?perObject=1.5`, `?perObject=Infinity` | a cap of 1.5 / clamped to 25 | `400`, `invalid_type` |
53+
| `GET /approvals/requests` | `?limit=abc`, `?limit=Infinity` | the unpaged 500-row list, no `total` | `400`, `invalid_type` |
54+
| `GET /approvals/requests` | `?limit=` (empty) | a one-row page | `400`, `invalid_type` |
55+
| `GET /approvals/requests` | `?offset=abc` | the first page | `400`, `invalid_type` |
56+
| `GET /approvals/requests` | `?offset=` (empty) | the service's 50-row paged mode | `400`, `invalid_type` |
57+
| `GET /approvals/requests` | `?limit=1.5`, `?offset=1.5` | 1.5 handed to the engine | `400`, `invalid_type` |
58+
59+
A blank value such as `?sinceSeq=%20` is refused on every one of these doors.
60+
61+
**Unchanged:**
62+
- An absent parameter keeps each door's default: the history log from the start, the
63+
audit trail's 100 events, previous-vs-current on `/diff`, search's 5 per object, and the
64+
unpaged approvals list.
65+
- An empty `?perObject=`, `?from=` or `?to=` still means absent, as it always did there.
66+
- Every conforming value reaches the service exactly as before, including the ranges no
67+
card here takes a position on: history still forwards `sinceSeq=0` or `1.5`, audit still
68+
forwards `limit=0` or `900` to its own clamp, `/diff` still forwards `from=0`, search
69+
`perObject=50` is still clamped to 25, and approvals `limit=0` / `offset=-1` still reach
70+
the service's own clamp.
71+
- `GET /data/:object/export?page=` is unchanged. It sets only the export's chunk size, and
72+
no value of it changes the rows exported.
73+
- `POST /meta/:type/:name/rollback` `toVersion` is unchanged. It already refused an
74+
unreadable value with `400 INVALID_REQUEST`.
75+
76+
**Fix for a caller that now gets the `400`:** send the parameter as a number (a whole
77+
number on `/diff`, search and approvals), or omit it to get the door's default.
78+
79+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authored moves: no spec key, metadata property or exported symbol is added, removed or renamed, and `packages/spec` is untouched, so `objectstack migrate meta` has nothing to rewrite and no ADR-0087 registry gains a row. What narrows is the set of HTTP query values five REST doors accept, back to what their declarations (`HistoryMetaItemRequestSchema`, `AuditMetaItemRequestSchema`) or a whole-number reading already said. A query string is neither authored nor persisted. The other categories are closed on facts: `@objectstack/rest` publishes (not `unpublished`), no ADR-0087 id covers an HTTP query value (not `registered` / `already-registered`), and the change is runtime behaviour, not a TypeScript declaration (not `runtime-interface-only` / `type-surface-only`). -->

0 commit comments

Comments
 (0)