diff --git a/.changeset/refuse-unrecognized-input.md b/.changeset/refuse-unrecognized-input.md new file mode 100644 index 00000000..c5ecffba --- /dev/null +++ b/.changeset/refuse-unrecognized-input.md @@ -0,0 +1,6 @@ +--- +'@haverstack/core': minor +'@haverstack/conformance-fixtures': minor +--- + +The `./wire` request parsers refuse input their endpoint does not define instead of ignoring it. `parseQueryParams()`, `parseChangeParams()` and `parseJournalParams()` throw `StackBadRequestError` for an unknown query param, a repeat of a param that names one value, or a boolean param other than `true`/`false`. `parseQueryBody()` does the same for an unknown key at any depth, a non-object body, a non-boolean `includeDeleted`/`includeUnlisted` or a non-string `cursor`. `createOptionsFromWireRecord()` refuses a key no wire record carries. The rule is specified in docs/spec/wire-format.md § Unrecognized input, and four new error fixtures cover it. diff --git a/docs/spec/wire-format.md b/docs/spec/wire-format.md index d3b40c94..96591c57 100644 --- a/docs/spec/wire-format.md +++ b/docs/spec/wire-format.md @@ -183,6 +183,18 @@ Empty is not an encoding of anything — nor is a literal `null`. Absence is `40 The distinction between **400** and **422** matters for write endpoints (`POST /records`, `PATCH /records/:id`, `POST /records/:id/migrate`, `POST /types`): a 400 covers a request that couldn't be parsed at all, or that is well-formed but addresses something that doesn't exist (an undefined `typeId`, a malformed TypeId, search text the engine cannot parse); a 422 means the server understood the request and what it addressed, but the content didn't satisfy the type schema. The distinguishing question is whether there was a schema to fail against: `StackValidationError.details` names the fields that broke, and an unknown type has no such detail to report — `Stack.create()` resolves the type and throws before the validation block runs for exactly this reason. +### Unrecognized input + +**A query param or JSON body key an endpoint does not define is refused with 400 (`bad_request`), never ignored.** Ignoring it answers a different request than the one sent, and the difference is always in the direction the caller didn't ask for: a misspelled filter widens a query, a misspelled `purge` soft-deletes, a misspelled field on a token request mints a token for someone else. A 400 tells the caller at once; a 200 for the wrong request never does. The rule holds at every depth of a body: a filter's `createdBy`, a sort, a relationship target. The same goes for values: a boolean param takes only `true` or `false`, and a param that names one value appears at most once, since reading a repeat as its first value ignores the rest. + +Three things sit outside it: + +- **Keys an endpoint defines as ignored are not unrecognized.** A create body is a whole record, so it may carry every key a wire record has, and the server-assigned ones are dropped as [Records](#records) requires rather than refused. +- **Headers.** A request carries headers the application never sees — proxies and browsers add them — so an unrecognized one is ignored, as is `If-Match` on the association endpoints (see [Records](#records)). +- **Responses.** A client reading a server's response ignores what it doesn't recognize, the way it ignores an [unrecognized frame](./change-feed.md). Strictness is the server's side of the contract, where the input is the client's intent. + +A server built on core reaches this through the wire parsers in `@haverstack/core/wire` — `parseQueryParams()`, `parseQueryBody()`, `parseChangeParams()`, `parseJournalParams()`, `createOptionsFromWireRecord()` and `changesFromWireBody()` — each of which refuses what its endpoint does not define. Endpoints a server parses itself — `/auth/token` among them — owe the same refusal. + ### The taxonomy root Every class in the table above extends the abstract `StackError`, so `err instanceof StackError` answers the one question a server's error middleware asks first: is this a Stack-domain failure with a wire representation, or an ordinary bug that should surface as a bare 500? Membership is exactly that guarantee — a `StackError` always has a `code`, and every code has a status. Errors with no wire mapping (`IdGenerationError`, `InvalidDidError`, `UseAfterCloseError`, `InvalidAdapterError`, `RelayScopeError`) stay outside the hierarchy for that reason. diff --git a/packages/adapter-api/tests/conformance.test.ts b/packages/adapter-api/tests/conformance.test.ts index 85020918..3c26cc11 100644 --- a/packages/adapter-api/tests/conformance.test.ts +++ b/packages/adapter-api/tests/conformance.test.ts @@ -741,10 +741,17 @@ const ERROR_CLASS_FOR_CODE = { /** * Fixtures no conformant client can originate: `QuerySort` is a union, so * a request naming both sort parameters is unrepresentable before it ever - * reaches the wire. The rule is a server's to enforce, and the fixture - * exists for server implementations to test against. + * reaches the wire, and APIAdapter never sends a name the wire does not + * define. The rules are a server's to enforce, and the fixtures exist for + * server implementations to test against. */ -const SERVER_ONLY_ERROR_FIXTURES = new Set(['error-bad-request-both-sort-parameters']); +const SERVER_ONLY_ERROR_FIXTURES = new Set([ + 'error-bad-request-both-sort-parameters', + 'error-bad-request-unknown-query-param', + 'error-bad-request-non-boolean-param', + 'error-bad-request-unknown-query-body-key', + 'error-bad-request-unknown-record-key', +]); describe('error response fixtures', () => { for (const fixture of errorResponseFixtures.filter( diff --git a/packages/conformance-fixtures/src/index.ts b/packages/conformance-fixtures/src/index.ts index a469c4e9..f2fb39bd 100644 --- a/packages/conformance-fixtures/src/index.ts +++ b/packages/conformance-fixtures/src/index.ts @@ -2136,6 +2136,66 @@ export const errorResponseFixtures: ConformanceFixture[] = [ }, }, }, + { + name: 'error-bad-request-unknown-query-param', + description: + 'A query param the endpoint does not define returns 400 with code "bad_request" rather ' + + 'than being ignored: an ignored filter param answers a wider query than the one sent, ' + + 'with a 200 that never says so. The same holds on every endpoint. See ' + + 'docs/spec/wire-format.md § Unrecognized input.', + method: 'GET', + path: '/records?authorId=did%3Akey%3Az6MkMember', + responseStatus: 400, + responseBody: { + error: { code: 'bad_request', message: 'Unknown query param: authorId' }, + }, + }, + { + name: 'error-bad-request-non-boolean-param', + description: + 'A boolean query param takes only "true" or "false"; any other value returns 400 with ' + + 'code "bad_request" rather than reading as false. See docs/spec/wire-format.md ' + + '§ Unrecognized input.', + method: 'GET', + path: '/records?includeDeleted=1', + responseStatus: 400, + responseBody: { + error: { + code: 'bad_request', + message: 'Invalid includeDeleted: expected true or false, got "1"', + }, + }, + }, + { + name: 'error-bad-request-unknown-query-body-key', + description: + 'POST /records/query refuses a key it does not define at any depth of the body — here ' + + 'inside filter.createdBy — with 400 / code "bad_request". See docs/spec/wire-format.md ' + + '§ Unrecognized input.', + method: 'POST', + path: '/records/query', + requestBody: { filter: { createdBy: { authorId: 'did:key:z6MkMember' } } }, + responseStatus: 400, + responseBody: { + error: { code: 'bad_request', message: 'Unknown key in filter.createdBy: authorId' }, + }, + }, + { + name: 'error-bad-request-unknown-record-key', + description: + 'POST /records accepts every key a wire record carries — dropping the server-assigned ' + + 'ones — and refuses any other with 400 / code "bad_request". See ' + + 'docs/spec/wire-format.md § Unrecognized input.', + method: 'POST', + path: '/records', + requestBody: { + typeId: 'com.example/note@1', + content: { title: 'Hello' }, + title: 'Hello', + }, + responseStatus: 400, + responseBody: { error: { code: 'bad_request', message: 'Unknown record key: title' } }, + }, { name: 'error-validation-failed', description: diff --git a/packages/core/src/wire-record.ts b/packages/core/src/wire-record.ts index 8145fce4..c25ca2f2 100644 --- a/packages/core/src/wire-record.ts +++ b/packages/core/src/wire-record.ts @@ -20,6 +20,7 @@ import type { AuthorityAssociation, DataAssociation, RecordChangeSet, + StackRecord, TokenSession, TypeId, } from './types.js'; @@ -35,6 +36,29 @@ export type WireCreateRequest = { options: Omit; }; +/** + * Every top-level key a wire record carries — a `StackRecord`'s, so a new + * field fails to compile here until it is listed. A create body is a whole + * record, so each of these is accepted — stamped ones are then dropped — + * and anything else is refused. See docs/spec/wire-format.md § Unrecognized input. + */ +const WIRE_RECORD_KEYS: readonly string[] = Object.keys({ + id: true, + typeId: true, + createdAt: true, + updatedAt: true, + content: true, + version: true, + parentId: true, + appId: true, + createdBy: true, + updatedBy: true, + deletedAt: true, + unlistedAt: true, + permissions: true, + associations: true, +} satisfies Record); + function requireBody(body: unknown): Record { if (typeof body !== 'object' || body === null || Array.isArray(body)) throw new StackBadRequestError('Invalid record body: expected an object'); @@ -94,6 +118,11 @@ export function createOptionsFromWireRecord( ownerEntityId: EntityId, ): WireCreateRequest { const record = requireBody(body); + const unknown = Object.keys(record).filter((key) => !WIRE_RECORD_KEYS.includes(key)); + if (unknown.length > 0) + throw new StackBadRequestError( + `Unknown record key${unknown.length > 1 ? 's' : ''}: ${unknown.join(', ')}`, + ); const typeId = record.typeId; if (typeof typeId !== 'string' || typeId === '') diff --git a/packages/core/src/wire-request.ts b/packages/core/src/wire-request.ts index bee7a1c6..59bfb01c 100644 --- a/packages/core/src/wire-request.ts +++ b/packages/core/src/wire-request.ts @@ -56,6 +56,76 @@ const SORT_FIELDS: ReadonlySet = new Set(NATIVE_SORT_FIELDS); const SORT_DIRECTIONS: ReadonlySet> = new Set(['asc', 'desc']); const CHANGE_KINDS: ReadonlySet = new Set(['created', 'changed', 'deleted', 'purged']); const TARGET_KINDS: ReadonlySet = new Set(['record', 'entity', 'external']); +const TARGET_KEYS: Record = { + record: ['kind', 'recordId', 'stackUrl'], + entity: ['kind', 'entityId'], + external: ['kind', 'ns', 'id'], +}; + +/** Every param `GET /records` defines. See docs/spec/wire-format.md § Records. */ +const RECORD_QUERY_PARAMS = [ + 'typeId', + 'parentId', + 'appId', + 'createdBySubject', + 'createdByPrincipal', + 'tag', + 'attachmentLabel', + 'attachmentFileId', + 'referencesFileId', + 'relatedTo', + 'relatedToStack', + 'relatedToEntity', + 'relatedToNs', + 'relatedToId', + 'relatedToLabel', + 'search', + 'createdBefore', + 'createdAfter', + 'updatedBefore', + 'updatedAfter', + 'includeDeleted', + 'includeUnlisted', + 'sort', + 'sortContent', + 'direction', + 'limit', + 'cursor', +] as const; + +const QUERY_BODY_FILTER_KEYS = [ + 'typeId', + 'parentId', + 'appId', + 'createdBy', + 'tags', + 'attachment', + 'referencesFileId', + 'relatedTo', + 'content', + 'contentPresent', + 'search', + 'includeDeleted', + 'includeUnlisted', + 'createdAt', + 'updatedAt', +] as const; + +/** + * Every param `GET /changes` defines. `since` is the resume cursor, which + * the server reads itself — see parseChangeParams(). + */ +const CHANGE_PARAMS = [ + 'typeId', + 'baseId', + 'parentId', + 'createdBySubject', + 'createdByPrincipal', + 'kind', + 'include', + 'includeUnlisted', + 'since', +] as const; /** * Strict positive-integer parse for a URL param — rejects "1abc", "2.7", @@ -138,6 +208,71 @@ function requirePlainObject(raw: unknown, label: string): Record; } +/** + * A plain object carrying only `keys`. An unrecognized key is refused + * rather than ignored, since ignoring it answers a different request than + * the one sent. See docs/spec/wire-format.md § Unrecognized input. + */ +function requireKnownKeys( + raw: unknown, + keys: readonly string[], + label: string, +): Record { + const obj = requirePlainObject(raw, label); + const unknown = Object.keys(obj).filter((key) => !keys.includes(key)); + if (unknown.length > 0) + throw new StackBadRequestError( + `Unknown key${unknown.length > 1 ? 's' : ''} in ${label}: ${unknown.join(', ')}`, + ); + return obj; +} + +/** + * Params a filter repeats to name several values. Any other param names + * one value, so a repeat of it is refused rather than read as its first. + */ +const REPEATABLE_PARAMS: ReadonlySet = new Set([ + 'typeId', + 'baseId', + 'appId', + 'createdBySubject', + 'createdByPrincipal', + 'tag', + 'kind', +]); + +/** The URL-param form of requireKnownKeys(), which also refuses a repeat. */ +function requireKnownParams(url: URL, names: readonly string[]): void { + const present = [...new Set(url.searchParams.keys())]; + const unknown = present.filter((name) => !names.includes(name)); + if (unknown.length > 0) + throw new StackBadRequestError( + `Unknown query param${unknown.length > 1 ? 's' : ''}: ${unknown.join(', ')}`, + ); + const repeated = present.filter( + (name) => !REPEATABLE_PARAMS.has(name) && url.searchParams.getAll(name).length > 1, + ); + if (repeated.length > 0) + throw new StackBadRequestError( + `Repeated query param${repeated.length > 1 ? 's' : ''}: ${repeated.join(', ')}`, + ); +} + +/** A boolean URL param: absent is false, and only `true`/`false` are values. */ +function booleanParam(url: URL, name: string): boolean { + const value = url.searchParams.get(name); + if (value === null || value === 'false') return false; + if (value === 'true') return true; + throw new StackBadRequestError(`Invalid ${name}: expected true or false, got "${value}"`); +} + +function optionalBoolean(raw: unknown, label: string): boolean | undefined { + if (raw === undefined) return undefined; + if (typeof raw !== 'boolean') + throw new StackBadRequestError(`Invalid ${label}: expected a boolean`); + return raw; +} + // ------------------------------------------------------- // Filter fields that never travel // ------------------------------------------------------- @@ -190,11 +325,11 @@ export function assertQueryTravels(query: StackQuery): void { * a kind it does not recognize. */ function parseRelatedToTarget(raw: unknown): RelationshipTargetPattern { - const t = requirePlainObject(raw, 'filter.relatedTo.target'); - if (typeof t.kind !== 'string' || !TARGET_KINDS.has(t.kind)) - throw new StackBadRequestError( - `Invalid filter.relatedTo.target.kind: ${JSON.stringify(t.kind)}`, - ); + const label = 'filter.relatedTo.target'; + const kind = requirePlainObject(raw, label).kind; + if (typeof kind !== 'string' || !TARGET_KINDS.has(kind)) + throw new StackBadRequestError(`Invalid filter.relatedTo.target.kind: ${JSON.stringify(kind)}`); + const t = requireKnownKeys(raw, TARGET_KEYS[kind as RelationshipTargetPattern['kind']], label); if (t.kind === 'record') { return { kind: 'record', @@ -219,7 +354,7 @@ function parseRelatedToTarget(raw: unknown): RelationshipTargetPattern { /** A label, a target, or both — the body form of the same filter. */ function parseRelatedToBody(raw: unknown): RelatedToFilter { - const r = requirePlainObject(raw, 'filter.relatedTo'); + const r = requireKnownKeys(raw, ['label', 'target'], 'filter.relatedTo'); const label = r.label !== undefined ? requireString(r.label, 'filter.relatedTo.label') : undefined; const target = r.target !== undefined ? parseRelatedToTarget(r.target) : undefined; @@ -295,7 +430,7 @@ function parseAttachmentParams(url: URL): AttachmentFilter | undefined { } function parseAttachmentBody(raw: unknown): AttachmentFilter { - const a = requirePlainObject(raw, 'filter.attachment'); + const a = requireKnownKeys(raw, ['label', 'fileId'], 'filter.attachment'); return { ...(a.label !== undefined && { label: requireString(a.label, 'filter.attachment.label') }), ...(a.fileId !== undefined && { fileId: requireString(a.fileId, 'filter.attachment.fileId') }), @@ -328,6 +463,7 @@ function parseCreatedByParams(url: URL): RecordFilter['createdBy'] { */ export function parseQueryParams(url: URL): StackQuery { assertNoUntravelableFields(url.searchParams.has('baseId'), url.searchParams.has('presentAt')); + requireKnownParams(url, RECORD_QUERY_PARAMS); const filter: RecordFilter = {}; @@ -376,8 +512,8 @@ export function parseQueryParams(url: URL): StackQuery { }; } - if (url.searchParams.get('includeDeleted') === 'true') filter.includeDeleted = true; - if (url.searchParams.get('includeUnlisted') === 'true') filter.includeUnlisted = true; + if (booleanParam(url, 'includeDeleted')) filter.includeDeleted = true; + if (booleanParam(url, 'includeUnlisted')) filter.includeUnlisted = true; const query: StackQuery = {}; if (Object.keys(filter).length) query.filter = filter; @@ -413,13 +549,17 @@ function parseLimitValue(raw: unknown): number { * Build a `StackQuery` from a `POST /records/query` JSON body — the * superset form, which additionally carries `filter.content`. Dates arrive * as the ISO strings `JSON.stringify` made of them and are decoded back to - * `Date`. See docs/spec/wire-format.md § Records. + * `Date`. `undefined` is the absent body; `null` or any other non-object + * is refused, so a server hands over no body as `undefined`. + * See docs/spec/wire-format.md § Records. */ export function parseQueryBody(raw: unknown): StackQuery { - if (!raw || typeof raw !== 'object') return {}; - const body = raw as Record; + if (raw === undefined) return {}; + const body = requirePlainObject(raw, 'query body'); const f = body.filter !== undefined ? requirePlainObject(body.filter, 'filter') : undefined; assertNoUntravelableFields(f !== undefined && 'baseId' in f, 'presentAt' in body); + requireKnownKeys(body, ['filter', 'sort', 'limit', 'cursor'], 'query body'); + if (f) requireKnownKeys(f, QUERY_BODY_FILTER_KEYS, 'filter'); const query: StackQuery = {}; @@ -431,7 +571,7 @@ export function parseQueryBody(raw: unknown): StackQuery { filter.parentId = f.parentId === null ? null : requireString(f.parentId, 'filter.parentId'); if (f.appId !== undefined) filter.appId = requireStringOrArray(f.appId, 'filter.appId'); if (f.createdBy !== undefined) { - const by = requirePlainObject(f.createdBy, 'filter.createdBy'); + const by = requireKnownKeys(f.createdBy, ['subjectId', 'principalId'], 'filter.createdBy'); filter.createdBy = { ...(by.subjectId !== undefined && { subjectId: requireStringOrArray(by.subjectId, 'filter.createdBy.subjectId'), @@ -450,18 +590,18 @@ export function parseQueryBody(raw: unknown): StackQuery { if (f.contentPresent !== undefined) filter.contentPresent = requireStringArray(f.contentPresent, 'filter.contentPresent'); if (f.search !== undefined) filter.search = requireString(f.search, 'filter.search'); - if (f.includeDeleted) filter.includeDeleted = true; - if (f.includeUnlisted) filter.includeUnlisted = true; + if (optionalBoolean(f.includeDeleted, 'filter.includeDeleted')) filter.includeDeleted = true; + if (optionalBoolean(f.includeUnlisted, 'filter.includeUnlisted')) filter.includeUnlisted = true; - if (f.createdAt) { - const r = requirePlainObject(f.createdAt, 'filter.createdAt'); + if (f.createdAt !== undefined) { + const r = requireKnownKeys(f.createdAt, ['before', 'after'], 'filter.createdAt'); filter.createdAt = { ...(r.before !== undefined && { before: requireDate(r.before, 'filter.createdAt.before') }), ...(r.after !== undefined && { after: requireDate(r.after, 'filter.createdAt.after') }), }; } - if (f.updatedAt) { - const r = requirePlainObject(f.updatedAt, 'filter.updatedAt'); + if (f.updatedAt !== undefined) { + const r = requireKnownKeys(f.updatedAt, ['before', 'after'], 'filter.updatedAt'); filter.updatedAt = { ...(r.before !== undefined && { before: requireDate(r.before, 'filter.updatedAt.before') }), ...(r.after !== undefined && { after: requireDate(r.after, 'filter.updatedAt.after') }), @@ -471,15 +611,15 @@ export function parseQueryBody(raw: unknown): StackQuery { query.filter = filter; } - if (body.sort) { - const s = requirePlainObject(body.sort, 'sort'); + if (body.sort !== undefined) { + const s = requireKnownKeys(body.sort, ['field', 'contentField', 'direction'], 'sort'); const sort = buildSort(s.field, s.contentField, s.direction); if (!sort) throw new StackBadRequestError('Invalid sort: expected a field or a contentField.'); query.sort = sort; } if (body.limit !== undefined) query.limit = parseLimitValue(body.limit); - if (typeof body.cursor === 'string') query.cursor = body.cursor; + if (body.cursor !== undefined) query.cursor = requireString(body.cursor, 'cursor'); return query; } @@ -506,6 +646,7 @@ export type ParsedChangeParams = { * the server's own resumption machinery rather than request encoding. */ export function parseChangeParams(url: URL): ParsedChangeParams { + requireKnownParams(url, CHANGE_PARAMS); const filter: ChangeFilter = {}; const typeIds = url.searchParams.getAll('typeId'); @@ -536,7 +677,7 @@ export function parseChangeParams(url: URL): ParsedChangeParams { return { filter, includeRecords: include === 'record', - includeUnlisted: url.searchParams.get('includeUnlisted') === 'true', + includeUnlisted: booleanParam(url, 'includeUnlisted'), }; } @@ -550,6 +691,7 @@ export function parseChangeParams(url: URL): ParsedChangeParams { * change feed's opaque cursor. See docs/spec/wire-format.md § Journal. */ export function parseJournalParams(url: URL): JournalQuery { + requireKnownParams(url, ['afterSeq', 'limit']); const query: JournalQuery = {}; const afterSeq = url.searchParams.get('afterSeq'); if (afterSeq !== null) query.afterSeq = parsePositiveInt(afterSeq, 'afterSeq'); diff --git a/packages/core/tests/wire-record.test.ts b/packages/core/tests/wire-record.test.ts index 941ee8c5..4b348e23 100644 --- a/packages/core/tests/wire-record.test.ts +++ b/packages/core/tests/wire-record.test.ts @@ -65,6 +65,13 @@ describe('createOptionsFromWireRecord — stamped fields', () => { }); }); +describe('createOptionsFromWireRecord — unrecognized keys', () => { + test('a key no wire record carries is refused rather than ignored', () => { + expect(() => parse(wireBody({ entityId: OWNER }))).toThrow(StackBadRequestError); + expect(() => parse(wireBody({ entityId: OWNER }))).toThrow(/entityId/); + }); +}); + // ------------------------------------------------------- // The clock fields // ------------------------------------------------------- diff --git a/packages/core/tests/wire-request.test.ts b/packages/core/tests/wire-request.test.ts index c4dcb61c..9c31b652 100644 --- a/packages/core/tests/wire-request.test.ts +++ b/packages/core/tests/wire-request.test.ts @@ -35,11 +35,27 @@ describe('parseQueryParams', () => { expect(parseQueryParams(url('')).filter).toBeUndefined(); }); - test('includeDeleted and includeUnlisted are set only by the literal "true"', () => { + test('includeDeleted and includeUnlisted take only "true" or "false"', () => { expect(parseQueryParams(url('?includeDeleted=true')).filter?.includeDeleted).toBe(true); - expect(parseQueryParams(url('?includeDeleted=1')).filter?.includeDeleted).toBeUndefined(); + expect(parseQueryParams(url('?includeDeleted=false')).filter?.includeDeleted).toBeUndefined(); expect(parseQueryParams(url('?includeUnlisted=true')).filter?.includeUnlisted).toBe(true); - expect(parseQueryParams(url('?includeUnlisted=yes')).filter?.includeUnlisted).toBeUndefined(); + expect(() => parseQueryParams(url('?includeDeleted=1'))).toThrow(StackBadRequestError); + expect(() => parseQueryParams(url('?includeUnlisted=yes'))).toThrow(StackBadRequestError); + }); + + test('an unrecognized param is refused rather than ignored', () => { + expect(() => parseQueryParams(url('?entityId=did:key:x'))).toThrow(/Unknown query param/); + }); + + test('a single-value param appears at most once; a filter list may repeat', () => { + expect(() => parseQueryParams(url('?includeDeleted=true&includeDeleted=junk'))).toThrow( + /Repeated query param: includeDeleted/, + ); + expect(() => parseQueryParams(url('?limit=5&limit=500'))).toThrow(/Repeated query param/); + expect(parseQueryParams(url('?typeId=a/b@1&typeId=a/c@1&tag=x&tag=y')).filter).toMatchObject({ + typeId: ['a/b@1', 'a/c@1'], + tags: ['x', 'y'], + }); }); test('a malformed date bound is refused rather than dropped', () => { @@ -145,10 +161,32 @@ describe('parseQueryParams', () => { // ------------------------------------------------------- describe('parseQueryBody', () => { - test('a non-object body parses to an empty query', () => { + test('an absent body is an empty query, and a non-object body is refused', () => { expect(parseQueryBody(undefined)).toEqual({}); - expect(parseQueryBody(null)).toEqual({}); - expect(parseQueryBody('nonsense')).toEqual({}); + expect(() => parseQueryBody(null)).toThrow(StackBadRequestError); + expect(() => parseQueryBody('nonsense')).toThrow(StackBadRequestError); + }); + + test('an unrecognized key is refused at every level', () => { + for (const body of [ + { filters: {} }, + { filter: { entityId: 'x' } }, + { filter: { createdBy: { entityId: 'x' } } }, + { filter: { attachment: { label: 'a', mime: 'x' } } }, + { filter: { createdAt: { before: '2024-01-01', on: '2024-01-01' } } }, + { filter: { relatedTo: { label: 'x', scope: 'entity' } } }, + { filter: { relatedTo: { target: { kind: 'entity', entityId: 'e', ns: 'x' } } } }, + { sort: { field: 'createdAt', order: 'asc' } }, + ]) { + expect(() => parseQueryBody(body), JSON.stringify(body)).toThrow(/Unknown key/); + } + }); + + test('a non-boolean includeDeleted or a non-string cursor is refused', () => { + expect(() => parseQueryBody({ filter: { includeDeleted: 'true' } })).toThrow( + StackBadRequestError, + ); + expect(() => parseQueryBody({ cursor: 5 })).toThrow(StackBadRequestError); }); test('ISO date strings decode back to Date objects', () => { @@ -319,9 +357,22 @@ describe('parseChangeParams', () => { expect(() => parseChangeParams(changes('?include=everything'))).toThrow(StackBadRequestError); }); - test('includeUnlisted is set only by the literal "true"', () => { + test('includeUnlisted takes only "true" or "false"', () => { expect(parseChangeParams(changes('?includeUnlisted=true')).includeUnlisted).toBe(true); - expect(parseChangeParams(changes('?includeUnlisted=1')).includeUnlisted).toBe(false); + expect(parseChangeParams(changes('?includeUnlisted=false')).includeUnlisted).toBe(false); + expect(() => parseChangeParams(changes('?includeUnlisted=1'))).toThrow(StackBadRequestError); + }); + + test('an unrecognized param is refused, and the resume cursor is not one', () => { + expect(() => parseChangeParams(changes('?token=abc'))).toThrow(/Unknown query param/); + expect(() => parseChangeParams(changes('?since=abc'))).not.toThrow(); + }); + + test('a single-value param appears at most once; a filter list may repeat', () => { + expect(() => parseChangeParams(changes('?include=record&include=record'))).toThrow( + /Repeated query param/, + ); + expect(() => parseChangeParams(changes('?kind=created&kind=deleted'))).not.toThrow(); }); }); @@ -404,6 +455,18 @@ describe('parseJournalParams', () => { expect(parseJournalParams(journal('/records/r1/journal'))).toEqual({}); }); + test('an unrecognized param is refused rather than ignored', () => { + expect(() => parseJournalParams(journal('/records/r1/journal?sinceSeq=1'))).toThrow( + /Unknown query param/, + ); + }); + + test('a repeated param is refused', () => { + expect(() => parseJournalParams(journal('/records/r1/journal?afterSeq=1&afterSeq=9'))).toThrow( + /Repeated query param/, + ); + }); + test('round-trips a window', () => { expect(parseJournalParams(journal('/records/r1/journal?afterSeq=4&limit=10'))).toEqual({ afterSeq: 4,