Skip to content

Commit 3dd76b2

Browse files
committed
fix(rest,runtime)!: a number expectedVersion on a /data PATCH is refused 400 on both doors
UpdateDataRequestSchema.expectedVersion declares a string. Both PATCH doors judged a body number as its String() and answered 409 CONCURRENT_UPDATE; RestServer's request gate only ever parsed the coerced copy. A number now joins the token shapes that can never carry a version: versionTokenRefusal (both copies, statement for statement) refuses it 400 VALIDATION_FAILED before the protocol is asked, and both doors gate and forward the token as the caller sent it. If-Match and the DELETE's ?expectedVersion are strings by transport and are unaffected. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLwgbTSmGMrqsXkBDsiwfG
1 parent 90424dd commit 3dd76b2

4 files changed

Lines changed: 154 additions & 32 deletions

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
'@objectstack/rest': minor
3+
'@objectstack/runtime': minor
4+
---
5+
6+
fix(rest,runtime)!: both `/data` write doors validate the request as the caller sent it: a `PATCH` or `POST` create whose body is not a record, and a `PATCH` whose `expectedVersion` is a number, are refused `400 VALIDATION_FAILED` on both doors before the engine is asked
7+
8+
`RestServer` validates `PATCH /data/:object/:id` against `UpdateDataRequestSchema` and `POST /data/:object` against `CreateDataRequestSchema`. The runtime dispatcher's `/data` domain, the only answer on a host that mounts just the `@objectstack/hono` catch-all, validated neither, so a body that is not a record reached the engine. On a `PATCH` it answered `500` with the engine's own error. On a `POST`, an array created one row per element and a number created an empty row, both answered `201`. `RestServer`'s own `PATCH` gate parsed a coerced copy of the request: it read a JSON `null` body as `{}`, and it read a numeric `expectedVersion` as its string. Both doors now validate the assembled request as the caller sent it, and they answer each case alike.
9+
10+
Clause-②: no
11+
12+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, object definition or stored shape is added, removed, renamed or re-shaped, and no exported declaration is added, removed or narrowed, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is the accept set of two HTTP routes on two doors: a write whose body is not the record the request schema declares, or whose body version token is a number where the schema declares a string, is now refused where it was applied or judged, and the remedy is in the request a caller sends, never a rewrite of a package source or a stored row. The other categories are closed on facts: both packages publish (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
13+
14+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
15+
16+
What changes for you, wherever `@objectstack/rest` or the runtime dispatcher serves these routes:
17+
18+
- `RestServer`, `PATCH /data/:object/:id` with a JSON `null` body: FROM `200`, an update that changed no field and still moved the record's `updated_at` (so every other holder of the old version saw a conflict on its next guarded save), TO `400 VALIDATION_FAILED` naming `data`, the record unchanged. The runtime dispatcher answers the same.
19+
- The runtime dispatcher, `PATCH /data/:object/:id` with an array, a string, a number or `null` as the body: FROM `500` carrying the engine's own error text, nothing written, TO `400 VALIDATION_FAILED` naming `data`, before the engine is asked. `RestServer` already answered `400` for the first three.
20+
- The runtime dispatcher, `POST /data/:object` with an array, a string or a number as the body: FROM `201` with one row created per array element, `201` with an empty row for a number, and the engine's own `400 INVALID_FIELD` for a string, TO `400 VALIDATION_FAILED` naming `data`, with no row created. `RestServer` already answered this.
21+
- Both doors, `PATCH /data/:object/:id` with a body `expectedVersion` that is a number (`5`, `0`): FROM judged as its string (`'5'`) and answered `409 CONCURRENT_UPDATE`, TO `400 VALIDATION_FAILED` before the protocol is asked, whatever `If-Match` is sent beside it. The request schema declares the token a string, and a number is now one of the token shapes that can never carry a version.
22+
23+
How it renders: `RestServer` answers `{ error, code: 'VALIDATION_FAILED', fields, object }`, the envelope its request gate already used. The runtime dispatcher returns its own `400 VALIDATION_FAILED` envelope, `fields` under `error.details`, instead of throwing, so a `@objectstack/hono` `createHonoApp` host renders it as `400`.
24+
25+
Unchanged: a record body, `{}` included; a string `expectedVersion` (judged; `409` on a mismatch); `If-Match` and the `DELETE`'s `?expectedVersion`, which are strings by transport; and a JSON `null` body on a `POST` create, which both doors still read as an empty record and answer `201`.
26+
27+
To send a patch or a row, send the fields as a JSON object. To guard a write, send the version you read (the record's `updated_at`) as a string. The `@objectstack/client` SDK's `data.update` sends the version as an `If-Match` header string, so an SDK caller is affected only when the `data` it passes is not an object, or carries a number `expectedVersion`.

‎packages/rest/src/rest-server.ts‎

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1743,17 +1743,25 @@ function sendMetaItemAbsent(res: any): void {
17431743
* - ABSENT: the spelling consulted is `undefined` or `null`. The body key or
17441744
* the query name falls through to `If-Match`, and no `If-Match` either is
17451745
* the legal unguarded write.
1746-
* - JUDGED: a string with something in it, or a finite number. The door
1747-
* forwards its string, and the protocol compares it (`409
1748-
* CONCURRENT_UPDATE` on a mismatch; the quoted-empty tag `""` is the
1749-
* protocol's own `400`, `MalformedVersionTokenError`).
1746+
* - JUDGED: a string with something in it. The door forwards it, and the
1747+
* protocol compares it (`409 CONCURRENT_UPDATE` on a mismatch; the
1748+
* quoted-empty tag `""` is the protocol's own `400`,
1749+
* `MalformedVersionTokenError`).
17501750
* - REFUSED `400 VALIDATION_FAILED`: anything else the caller put in a token
1751-
* position — an empty or blank string, a boolean, an array, an object. It
1752-
* can never carry a version, so it is the protocol's ruled answer to `""`
1753-
* (maintainer decision #20 ①: a token shape that can never carry a version
1754-
* is refused, neither read as "no token" nor answered `409`), given here
1755-
* because the protocol never sees these: it reads a blank string as no
1756-
* token.
1751+
* position — an empty or blank string, a number, a boolean, an array, an
1752+
* object. It can never carry a version, so it is the protocol's ruled
1753+
* answer to `""` (maintainer decision #20 ①: a token shape that can never
1754+
* carry a version is refused, neither read as "no token" nor answered
1755+
* `409`), given here because the protocol never sees these: it reads a
1756+
* blank string as no token.
1757+
*
1758+
* [#23517] A number is in the refused set, no longer judged as its string.
1759+
* `UpdateDataRequestSchema.expectedVersion` declares a string, and the door
1760+
* forwarded `String(5)`, so the request gate below only ever parsed the
1761+
* coerced copy and a token the contract never admits was answered `409` as if
1762+
* it were a stale version. Only a body token can be a number: `If-Match` and
1763+
* `?expectedVersion` are strings by transport, so the `DELETE` and every
1764+
* header token are unaffected.
17571765
*
17581766
* What it replaces is a truthiness test. The door forwarded a token only when
17591767
* it was truthy, and `bodyVersion ?? If-Match` reads `If-Match` only when the
@@ -1779,14 +1787,13 @@ function sendMetaItemAbsent(res: any): void {
17791787
function versionTokenRefusal(token: unknown, spelling: string): string | undefined {
17801788
if (token === undefined || token === null) return undefined;
17811789
if (typeof token === 'string' && token.trim() !== '') return undefined;
1782-
if (typeof token === 'number' && Number.isFinite(token)) return undefined;
17831790
const shape = typeof token === 'string' ? 'empty'
17841791
: Array.isArray(token) ? 'an array'
17851792
: typeof token === 'object' ? 'an object'
17861793
: `the ${typeof token} \`${String(token)}\``;
17871794
return `${spelling} is ${shape}, which can never carry a version, so it is refused rather than read as `
1788-
+ 'no token. Send the version you read (the record\'s `updated_at`), or omit both expectedVersion and '
1789-
+ 'If-Match to perform an unguarded write.';
1795+
+ 'no token. Send the version you read as a string (the record\'s `updated_at`), or omit both '
1796+
+ 'expectedVersion and If-Match to perform an unguarded write.';
17901797
}
17911798

17921799
/**
@@ -9182,12 +9189,20 @@ export class RestServer {
91829189
// like an array, a string or a number, and the runtime
91839190
// dispatcher's twin (`domains/data.ts`) asks this gate
91849191
// over the same assembled request.
9192+
//
9193+
// [#23517] …and the token as the caller sent it, never
9194+
// its `String()`: this gate parsed `String(5)`, so a
9195+
// number `expectedVersion: z.string()` declares no room
9196+
// for was forwarded and judged `409`.
9197+
// `versionTokenRefusal` above already refuses it, so what
9198+
// reaches this gate and the forward below is a non-blank
9199+
// string.
91859200
const { UpdateDataRequestSchema } = await import('@objectstack/spec/api');
91869201
const updateInput = {
91879202
object: req.params.object,
91889203
id: req.params.id,
91899204
data,
9190-
...(expectedVersion != null ? { expectedVersion: String(expectedVersion) } : {}),
9205+
...(expectedVersion != null ? { expectedVersion } : {}),
91919206
};
91929207
const parsedUpdateOne = (UpdateDataRequestSchema as any).safeParse(updateInput);
91939208
if (!parsedUpdateOne.success) {
@@ -9203,7 +9218,7 @@ export class RestServer {
92039218
object: req.params.object,
92049219
id: req.params.id,
92059220
data,
9206-
...(expectedVersion != null ? { expectedVersion: String(expectedVersion) } : {}),
9221+
...(expectedVersion != null ? { expectedVersion } : {}),
92079222
...(environmentId ? { environmentId } : {}),
92089223
...(context ? { context } : {}),
92099224
};

‎packages/runtime/src/domains/data-update-expected-version.test.ts‎

Lines changed: 76 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -294,12 +294,12 @@ for (const [doorName, boot] of DOORS) {
294294
* row. What either door does with them is `RestServer`'s reading, not this
295295
* file's: a token is the body key when it is not nullish (else `If-Match`),
296296
* the key is stripped from the patch whatever its value, and a token is
297-
* forwarded as its string unless it can never carry a version, which is
298-
* refused (#23492, pinned in its own block below).
297+
* forwarded unless it can never carry a version, which is refused (#23492,
298+
* pinned in its own block below; a number among them since #23517, pinned in
299+
* the block after it).
299300
*/
300301
const EDGE_SHAPES: ReadonlyArray<readonly [string, (version: string) => PatchOptions]> = [
301302
['a stale token', () => ({ body: { name: AFTER, expectedVersion: STALE } })],
302-
['a number token', () => ({ body: { name: AFTER, expectedVersion: 20000101 } })],
303303
['a null body token beside a stale If-Match', () => ({ body: { name: AFTER, expectedVersion: null }, headers: { 'If-Match': STALE } })],
304304
['a null body token beside the current If-Match', (v) => ({ body: { name: AFTER, expectedVersion: null }, headers: { 'If-Match': v } })],
305305
['an empty-string body token beside a stale If-Match', () => ({ body: { name: AFTER, expectedVersion: '' }, headers: { 'If-Match': STALE } })],
@@ -346,7 +346,8 @@ describe('[#23462] the two doors answer each token shape with the same status, c
346346
* position that can never carry a version is refused with the protocol's own
347347
* answer to the quoted-empty tag `""`, `400 VALIDATION_FAILED`, before
348348
* `updateData` is asked; maintainer decision #20 ① rules that shape refused,
349-
* neither read as "no token" nor answered `409`.
349+
* neither read as "no token" nor answered `409`. A number joined this set
350+
* with #23517 and has its own block below.
350351
*/
351352
const UNUSABLE: ReadonlyArray<readonly [string, PatchOptions]> = [
352353
["a body expectedVersion of '' beside a stale If-Match", { body: { name: AFTER, expectedVersion: '' }, headers: { 'If-Match': STALE } }],
@@ -391,10 +392,13 @@ for (const [doorName, boot] of DOORS) {
391392
expect(await door.name()).toBe(BEFORE);
392393
}, 60_000);
393394

394-
it('control: a body token of 0 is a number, judged as "0" (refused 409), never dropped', async () => {
395+
it('control: a body token of 0 is never dropped: it is refused 400, so the current If-Match beside it is not read', async () => {
396+
// [#23517] It was judged as '0' (409) until a number joined the
397+
// refused set. Either way it is not read as "no token": had it
398+
// been dropped, the current If-Match would have applied the write.
395399
const door = await boot();
396-
conflict(await door.patch({ body: { name: AFTER, expectedVersion: 0 }, headers: { 'If-Match': door.version } }));
397-
expect(door.forwarded()).toEqual([{ expectedVersion: '0', data: { name: AFTER } }]);
400+
refused(await door.patch({ body: { name: AFTER, expectedVersion: 0 }, headers: { 'If-Match': door.version } }));
401+
expect(door.forwarded()).toEqual([]);
398402
expect(await door.name()).toBe(BEFORE);
399403
}, 60_000);
400404
});
@@ -411,6 +415,71 @@ describe('[#23492] the two doors refuse each token that can never carry a versio
411415
}
412416
});
413417

418+
// ── A number token ────────────────────────────────────────────────────────────
419+
420+
/**
421+
* [#23517] `UpdateDataRequestSchema.expectedVersion` declares a string. Both
422+
* doors forwarded a number as `String(token)`, so `RestServer`'s request gate
423+
* only ever parsed the coerced copy and the dispatcher had no gate: a body
424+
* `expectedVersion: 5` was judged as `'5'` and answered `409
425+
* CONCURRENT_UPDATE` on both doors (measured on this rig before the fix,
426+
* beside a stale and beside the current `If-Match`; the row unchanged). Now a
427+
* number is refused `400 VALIDATION_FAILED` before `updateData` is asked, the
428+
* answer maintainer decision #20 ① gives a token shape that can never carry a
429+
* version. The `If-Match` beside it is never read: the body key is the token
430+
* whenever it is not nullish.
431+
*/
432+
const NUMBER_TOKENS: ReadonlyArray<readonly [string, number]> = [['5', 5], ['0', 0]];
433+
const IF_MATCH_BESIDE: ReadonlyArray<readonly [string, (version: string) => string]> = [
434+
['a stale If-Match', () => STALE],
435+
['the current If-Match', (version) => version],
436+
];
437+
438+
for (const [doorName, boot] of DOORS) {
439+
describe(`[#23517] ${doorName}: PATCH /data/:object/:id refuses a number expectedVersion`, () => {
440+
for (const [tokenName, token] of NUMBER_TOKENS) {
441+
for (const [besideName, ifMatch] of IF_MATCH_BESIDE) {
442+
it(`expectedVersion: ${tokenName} beside ${besideName}: 400 VALIDATION_FAILED, updateData never asked, the row unchanged`, async () => {
443+
const door = await boot();
444+
refused(await door.patch({ body: { name: AFTER, expectedVersion: token }, headers: { 'If-Match': ifMatch(door.version) } }));
445+
expect(door.forwarded()).toEqual([]);
446+
expect(await door.name()).toBe(BEFORE);
447+
}, 60_000);
448+
}
449+
}
450+
451+
it('control: the same token as a string is judged, not refused (409 against a stale version)', async () => {
452+
const door = await boot();
453+
conflict(await door.patch({ body: { name: AFTER, expectedVersion: '5' }, headers: { 'If-Match': door.version } }));
454+
expect(door.forwarded()).toEqual([{ expectedVersion: '5', data: { name: AFTER } }]);
455+
expect(await door.name()).toBe(BEFORE);
456+
}, 60_000);
457+
458+
it('control: If-Match alone is a string by transport, so the current one still updates', async () => {
459+
const door = await boot();
460+
updated(await door.patch({ body: { name: AFTER }, headers: { 'If-Match': door.version } }));
461+
expect(door.forwarded()).toEqual([{ expectedVersion: door.version, data: { name: AFTER } }]);
462+
expect(await door.name()).toBe(AFTER);
463+
}, 60_000);
464+
});
465+
}
466+
467+
describe('[#23517] the two doors refuse a number expectedVersion alike', () => {
468+
for (const [tokenName, token] of NUMBER_TOKENS) {
469+
for (const [besideName, ifMatch] of IF_MATCH_BESIDE) {
470+
it(`expectedVersion: ${tokenName} beside ${besideName}: the same status, code and sentence`, async () => {
471+
const answers: Answer[] = [];
472+
for (const [, boot] of DOORS) {
473+
const door = await boot();
474+
answers.push(await door.patch({ body: { name: AFTER, expectedVersion: token }, headers: { 'If-Match': ifMatch(door.version) } }));
475+
}
476+
const [rest, catchAllAnswer] = answers;
477+
expect(catchAllAnswer).toEqual(rest);
478+
}, 60_000);
479+
}
480+
}
481+
});
482+
414483
// ── The dispatcher's fallback updater ─────────────────────────────────────────
415484

416485
/**

0 commit comments

Comments
 (0)