Skip to content

Commit dd5daac

Browse files
authored
fix(data): reject unknown list query params instead of reading them as zero-matching filters (#4134) (#4169)
`GET /api/v1/data/:object` reads any parameter it does not reserve as a field-level equality filter. When the name matched no field, the predicate could only ever match nothing, so `?pageSize=5` returned 200 + `total: 0` — indistinguishable from an empty object, while the write path already rejected the same unknown name with `INVALID_FIELD`. `findData` now validates leftover parameters against the object's field map before lowering them into implicit filters, and throws the write path's own envelope: 400 `INVALID_FIELD` + `field` + `object`, with a suggestion (the canonical parameter for a known dialect, or the closest field name for a typo). The check runs before the `!options.where` branch, so the rejection does not depend on whether an explicit `filter` rode along. The reserved set is `Record<keyof QueryAST, true>`-pinned to the spec. The known-field-plus-explicit-filter case is left alone and filed as #4164.
1 parent efcd68c commit dd5daac

8 files changed

Lines changed: 797 additions & 22 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/rest": patch
4+
---
5+
6+
fix(data): reject unknown list query parameters instead of reading them as zero-matching field filters (#4134)
7+
8+
`GET /api/v1/data/:object` reads any parameter it does not reserve as a
9+
field-level equality filter — that is what makes `?status=done` shorthand for
10+
`?filter={"status":"done"}`. When the name matched **no** field the resulting
11+
predicate could only ever match nothing, so `?pageSize=5` on a 10-row object
12+
returned `200` + `total: 0`: structurally valid, and indistinguishable from
13+
"this object is empty". The write path already rejected the same unknown name
14+
loudly (`400 INVALID_FIELD`), so one piece of knowledge — does this field
15+
exist — was enforced on write and silently zeroed on read.
16+
17+
The read path now answers the same way, in the same envelope:
18+
19+
```json
20+
{
21+
"error": "Unknown field 'pageSize' on object 'showcase_task'. Query parameters that are not reserved are read as field filters, so an unknown name can only match zero records. Did you mean the 'top' query parameter (OData spelling '$top')?",
22+
"code": "INVALID_FIELD",
23+
"field": "pageSize",
24+
"object": "showcase_task"
25+
}
26+
```
27+
28+
The rejection carries a suggestion — the canonical parameter for a known
29+
dialect (`pageSize` / `perPage` / `page` / `sortBy` / `q``top` / `skip` /
30+
`sort` / `search`), or the closest real field name when it reads like a typo —
31+
and fires whether or not an explicit `filter` rode along, so the failure never
32+
depends on which other parameters were sent.
33+
34+
**What changes for callers:** a request sending a parameter that names no field
35+
now gets a `400` where it used to get an empty `200`. Page size is `top` /
36+
`$top` / `limit`; page offset is `skip` / `$skip` / `offset`. Every documented
37+
parameter, every `$`-prefixed OData alias, and the full `QueryAST` body of
38+
`POST /data/:object/query` are unaffected. An object with a field named after a
39+
reserved parameter (`count`, `cursor`, `object`, `top`, `search`, …) filters it
40+
through the explicit form: `?filter={"count":3}`.

content/docs/api/data-api.mdx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,35 @@ Query records with filtering, sorting, selection, and pagination.
2828

2929
> **Note:** OData-style `$`-prefixed parameters (`$filter`, `$select`, `$orderby`, `$top`, `$skip`, `$expand`, `$count`, `$search`) are also accepted directly on this same endpoint as aliases — they're normalized internally to the parameter names above. There is no separate standalone OData endpoint.
3030
31+
#### Any other parameter is a field filter — and must name a real field
32+
33+
A query parameter the endpoint does not reserve is read as a field-level
34+
equality filter, so `?status=done` is shorthand for
35+
`?filter={"status":"done"}`. Because such a parameter *is* a predicate, one
36+
naming a field the object does not have could only ever match zero records —
37+
so the endpoint rejects it instead of returning an empty page:
38+
39+
```http
40+
GET /api/v1/data/showcase_task?pageSize=5
41+
```
42+
```json
43+
{
44+
"error": "Unknown field 'pageSize' on object 'showcase_task'. Query parameters that are not reserved are read as field filters, so an unknown name can only match zero records. Did you mean the 'top' query parameter (OData spelling '$top')?",
45+
"code": "INVALID_FIELD",
46+
"field": "pageSize",
47+
"object": "showcase_task"
48+
}
49+
```
50+
51+
This is the same `400 INVALID_FIELD` the write path returns for an unknown
52+
field name, and it applies whether or not an explicit `filter` rode along.
53+
Page size is `top` / `$top` / `limit``pageSize`, `page_size` and `perPage`
54+
are not accepted spellings.
55+
56+
Reserved names cannot double as implicit filters. An object with a field
57+
literally called `count`, `cursor`, `distinct`, `object`, `search` or `top`
58+
filters it through the explicit form (`?filter={"count":3}`).
59+
3160
**Response**:
3261
```json
3362
{

content/docs/api/error-catalog.mdx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,14 @@ ObjectStack uses a structured error system with **9 error categories** and **53
7171
```
7272

7373
### `INVALID_FIELD`
74-
**Cause:** A field name in the request does not exist on the target object.
75-
**Fix:** Check the object schema for valid field names. Use `os meta get object <name>` to inspect the object's fields.
74+
**Cause:** A field name in the request does not exist on the target object. On a
75+
list read this also covers an unreserved query parameter — `GET /data/:object`
76+
reads those as field filters, so one naming no field could only match zero
77+
records and is rejected rather than answered with an empty page.
78+
**Fix:** Check the object schema for valid field names. Use `os meta get object <name>` to inspect the object's fields. If the name was meant as a
79+
*parameter* rather than a field, use the real one — page size is `top` / `$top`
80+
/ `limit`, not `pageSize` / `perPage`; the response's `error` names the
81+
substitute. See the [Data API](/docs/api/data-api).
7682
**Retry:** `no_retry`
7783

7884
### `MISSING_REQUIRED_FIELD`

packages/metadata-protocol/src/protocol.ts

Lines changed: 232 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type {
1818
} from '@objectstack/spec/api';
1919
import type { MetadataCacheRequest, MetadataCacheResponse, ServiceInfo, ApiRoutes, WellKnownCapabilities } from '@objectstack/spec/api';
2020
import { readServiceSelfInfo } from '@objectstack/spec/api';
21-
import { parseFilterAST, isFilterAST, type DroppedFieldsEvent } from '@objectstack/spec/data';
21+
import { parseFilterAST, isFilterAST, type DroppedFieldsEvent, type QueryAST } from '@objectstack/spec/data';
2222
import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared';
2323
import { type FormView, isAggregatedViewContainer } from '@objectstack/spec/ui';
2424
import { METADATA_FORM_REGISTRY } from '@objectstack/spec/system';
@@ -712,6 +712,149 @@ function mergeDroppedFieldEvents(events: DroppedFieldsEvent[]): DroppedFieldsEve
712712
return Array.from(byKey.values()).map((b) => ({ object: b.object, fields: Array.from(b.fields), reason: b.reason }));
713713
}
714714

715+
/**
716+
* The canonical `QueryAST` surface (`spec/data/query.zod.ts`), enumerated.
717+
*
718+
* Typed as `Record<keyof QueryAST, true>` so `tsc` pins it to the spec in BOTH
719+
* directions: a key added there is a missing-property error here, a key removed
720+
* there is an excess-property error here. That matters because the set below
721+
* decides what is a query parameter and what is a field filter — silently
722+
* drifting from the AST would resurrect exactly the #4134 failure for whatever
723+
* key was added.
724+
*/
725+
const QUERY_AST_KEYS: Readonly<Record<keyof QueryAST, true>> = {
726+
object: true, fields: true, where: true, search: true, orderBy: true,
727+
limit: true, offset: true, top: true, cursor: true, joins: true,
728+
aggregations: true, groupBy: true, having: true, windowFunctions: true,
729+
distinct: true, expand: true,
730+
};
731+
732+
/**
733+
* [#4134] Every query-parameter name `findData` consumes itself, consulted
734+
* AFTER the alias normalization in `findData` has run — so the wire spellings
735+
* that get rewritten (`$top`→`top`→`limit`, `select`→`fields`, `sort`→
736+
* `orderBy`, `filter`/`filters`/`$filter`→`where`, `populate`/`$expand`→
737+
* `expand`, `skip`→`offset`, …) are already gone by this point and
738+
* deliberately do NOT appear here. Anything still standing is either a name in
739+
* this set or a candidate field filter.
740+
*
741+
* The structural AST keys (`object`, `joins`, `having`, `windowFunctions`)
742+
* matter even though no querystring carries them: `POST /data/:object/query`
743+
* hands its body in as `query`, and that body IS a `Partial<QueryAST>`. Without
744+
* them, `client.data.query('task', { object: 'task', limit: 5 })` would have
745+
* its `object` key read as a filter and match zero rows.
746+
*
747+
* A name in this set can never be used as an implicit field filter, so an
748+
* object with a field genuinely called e.g. `count` or `cursor` must filter it
749+
* through the explicit form (`?filter={"count":3}`). That trade-off predates
750+
* #4134 for the original members; it is called out here so the next person to
751+
* add one knows what they are spending.
752+
*/
753+
const RESERVED_LIST_QUERY_PARAMS: ReadonlySet<string> = new Set([
754+
...Object.keys(QUERY_AST_KEYS),
755+
// Transport-only extras the normalizer consumes but the AST does not name.
756+
'count', // ?count / $count — response flag, not a projection
757+
'searchFields', // ?searchFields / $searchFields — ADR-0061 override
758+
// Server-derived, never caller input (stripped then re-set from `request`).
759+
'context',
760+
]);
761+
762+
/**
763+
* [#4134] High-frequency wrong guesses → the parameter that actually works.
764+
* Keys are normalized (lower-cased, `_`/`-` stripped) so `pageSize`,
765+
* `page_size` and `PAGE-SIZE` all land on the same entry.
766+
*
767+
* This is a HINT table, not an alias table: nothing here is accepted as input.
768+
* Adding an entry makes a rejection more helpful; it never makes a request
769+
* succeed, so it does not create the second de-facto contract Prime Directive
770+
* #12 warns about.
771+
*/
772+
const QUERY_PARAM_NEAR_MISS: Readonly<Record<string, string>> = {
773+
// page-size dialects (the #4134 repro: `?pageSize=5` → 200 + empty list)
774+
pagesize: 'top', persize: 'top', perpage: 'top', pagelimit: 'top',
775+
rowsperpage: 'top', pagecount: 'top', size: 'top', take: 'top',
776+
first: 'top', max: 'top', maxresults: 'top', maxrecords: 'top',
777+
// page-offset dialects
778+
page: 'skip', pageno: 'skip', pagenum: 'skip', pagenumber: 'skip',
779+
pageindex: 'skip', start: 'skip', startindex: 'skip', startat: 'skip',
780+
// sorting
781+
sortby: 'sort', sortfield: 'sort', sortorder: 'sort', order: 'sort',
782+
ordering: 'sort',
783+
// search
784+
q: 'search', keyword: 'search', keywords: 'search', term: 'search',
785+
searchterm: 'search', querytext: 'search',
786+
// filtering
787+
filterby: 'filter', criteria: 'filter', conditions: 'filter',
788+
// projection / relations
789+
columns: 'select', include: 'expand', includes: 'expand', with: 'expand',
790+
};
791+
792+
/**
793+
* The OData spelling of each parameter {@link QUERY_PARAM_NEAR_MISS} points at.
794+
* NOT derivable by prefixing `$` — `sort` is `$orderby`, and suggesting a
795+
* `$sort` that the unsupported-`$` guard rejects would just hand the caller a
796+
* second 400.
797+
*/
798+
const ODATA_SPELLING: Readonly<Record<string, string>> = {
799+
top: '$top', skip: '$skip', sort: '$orderby', search: '$search',
800+
filter: '$filter', select: '$select', expand: '$expand',
801+
};
802+
803+
/** Fold a parameter name to its near-miss lookup key. */
804+
function nearMissKey(name: string): string {
805+
return name.toLowerCase().replace(/[_-]/g, '');
806+
}
807+
808+
/** Levenshtein distance, bailed out early once it exceeds `max`. */
809+
function editDistance(a: string, b: string, max: number): number {
810+
if (Math.abs(a.length - b.length) > max) return max + 1;
811+
let prev = Array.from({ length: b.length + 1 }, (_, i) => i);
812+
for (let i = 1; i <= a.length; i++) {
813+
const row = [i];
814+
let best = i;
815+
for (let j = 1; j <= b.length; j++) {
816+
const cost = a[i - 1] === b[j - 1] ? 0 : 1;
817+
row[j] = Math.min(prev[j] + 1, row[j - 1] + 1, prev[j - 1] + cost);
818+
if (row[j] < best) best = row[j];
819+
}
820+
if (best > max) return max + 1;
821+
prev = row;
822+
}
823+
return prev[b.length];
824+
}
825+
826+
/**
827+
* [#4134] Actionable tail for an unknown list query parameter: the canonical
828+
* spelling when the caller used a known dialect (`pageSize` → `$top`), else the
829+
* closest real field name when it reads like a typo (`stauts` → `status`).
830+
* Returns `''` when nothing is close enough to be worth guessing.
831+
*/
832+
function suggestQueryParam(param: string, knownFields: readonly string[]): string {
833+
const canonical = QUERY_PARAM_NEAR_MISS[nearMissKey(param)];
834+
if (canonical) {
835+
const odata = ODATA_SPELLING[canonical];
836+
return ` Did you mean the '${canonical}' query parameter`
837+
+ (odata ? ` (OData spelling '${odata}')` : '')
838+
+ '?';
839+
}
840+
const folded = nearMissKey(param);
841+
// Only worth guessing for names long enough that a small distance is
842+
// meaningful — at 3 chars everything is within 2 edits of everything.
843+
if (folded.length >= 4) {
844+
const max = folded.length <= 5 ? 1 : 2;
845+
let best: string | undefined;
846+
let bestDistance = max + 1;
847+
for (const field of knownFields) {
848+
const d = editDistance(folded, nearMissKey(field), max);
849+
if (d < bestDistance) { bestDistance = d; best = field; }
850+
}
851+
if (best !== undefined && bestDistance <= max) {
852+
return ` Did you mean the field '${best}'?`;
853+
}
854+
}
855+
return '';
856+
}
857+
715858
/**
716859
* Service Configuration for Discovery
717860
* Maps service names to their routes and plugin providers.
@@ -2781,6 +2924,57 @@ export class ObjectStackProtocolImplementation implements
27812924
throw err;
27822925
}
27832926

2927+
/**
2928+
* [#4134] Read-path unknown-field gate for the implicit filters `findData`
2929+
* derives from leftover query parameters.
2930+
*
2931+
* Rejects with the SAME envelope the write path produces for the same
2932+
* mistake — `400 INVALID_FIELD` + `field` + `object` (see `mapDataError` in
2933+
* `@objectstack/rest`) — so "does this field exist" has one answer on both
2934+
* sides of the API instead of being enforced on write and silently
2935+
* zeroed on read.
2936+
*
2937+
* Tiering mirrors {@link assertObjectRegistered}, one level down:
2938+
*
2939+
* - **Schema present with a field map → authoritative.** An unlisted name
2940+
* is a 400. The registry injects the audit/tenant/owner columns
2941+
* (`created_at`, `created_by`, `updated_at`, `updated_by`,
2942+
* `organization_id`, `owner_id`) into `fields`, so those filter normally;
2943+
* `id` is added here because it is the primary key rather than a declared
2944+
* field. Dotted paths are judged on their head segment only
2945+
* (`owner_id.name`), matching how `engine.find()` validates projections.
2946+
* - **No registry, or a schema with no field map → skip.** Nothing to check
2947+
* against (registry-less Lite/edge hosts, engine doubles, external
2948+
* datasources whose columns are not mirrored locally). The
2949+
* object-existence gate above already warns once when the registry itself
2950+
* is missing, so this stays quiet rather than warning twice per process.
2951+
*/
2952+
private assertQueryParamsAreFields(object: string, params: readonly string[]): void {
2953+
const schema: any = this.engine?.registry?.getObject?.(object);
2954+
const declared = schema?.fields;
2955+
if (!declared || typeof declared !== 'object') return;
2956+
const fieldNames = Object.keys(declared);
2957+
if (fieldNames.length === 0) return;
2958+
const known = new Set(fieldNames);
2959+
known.add('id');
2960+
const unknown = params.filter((p) => !known.has(String(p).split('.')[0]));
2961+
if (unknown.length === 0) return;
2962+
const first = unknown[0];
2963+
const err: any = new Error(
2964+
`Unknown field '${first}' on object '${object}'`
2965+
+ (unknown.length > 1 ? ` (also: ${unknown.slice(1).join(', ')})` : '')
2966+
+ '. Query parameters that are not reserved are read as field filters, so an '
2967+
+ 'unknown name can only match zero records.'
2968+
+ suggestQueryParam(first, fieldNames),
2969+
);
2970+
err.code = 'INVALID_FIELD';
2971+
err.status = 400;
2972+
err.field = first;
2973+
err.fields = unknown;
2974+
err.object = object;
2975+
throw err;
2976+
}
2977+
27842978
async findData(request: { object: string, query?: any, context?: any }) {
27852979
// [#3770] Existence first: an unregistered object is a 404 before any
27862980
// query parameter is even parsed, so an unknown name can never be
@@ -2849,8 +3043,12 @@ export class ObjectStackProtocolImplementation implements
28493043
}
28503044
if (options.skip != null) {
28513045
options.offset = Number(options.skip);
2852-
delete options.skip;
28533046
}
3047+
// Deleted unconditionally, unlike `top` (a declared QueryAST key the
3048+
// engine aliases itself): `skip` is wire-only, so a null/undefined one
3049+
// left behind would reach the #4134 field gate below and be reported as
3050+
// an unknown FIELD — a confusing rejection for a real parameter.
3051+
delete options.skip;
28543052
if (options.limit != null) options.limit = Number(options.limit);
28553053
if (options.offset != null) options.offset = Number(options.offset);
28563054

@@ -2965,32 +3163,48 @@ export class ObjectStackProtocolImplementation implements
29653163
throw err;
29663164
}
29673165

3166+
// [#4134] A leftover key is about to be lowered into a field-equality
3167+
// predicate (or, when an explicit `where` won, dropped on the floor).
3168+
// Both readings are only sound if the key names a REAL field: a filter
3169+
// on a field that does not exist can never match, so `?pageSize=5`
3170+
// returned 200 + `total: 0` — indistinguishable from "no data". The
3171+
// write path already rejects the same input loudly (`INVALID_FIELD`);
3172+
// this is the read half of that one piece of knowledge, and the mirror
3173+
// of #3948's rule for driver-memory: an unapplied filter must not look
3174+
// like a satisfied one.
3175+
//
3176+
// Validated BEFORE the `!options.where` branch below, so a bad param is
3177+
// a 400 whether or not the caller also sent an explicit filter — the
3178+
// failure must not depend on which other params rode along.
3179+
const leftoverParams = Object.keys(options).filter((k) => !RESERVED_LIST_QUERY_PARAMS.has(k));
3180+
if (leftoverParams.length > 0) {
3181+
this.assertQueryParamsAreFields(request.object, leftoverParams);
3182+
}
3183+
29683184
// Flat field filters: REST-style query params like ?id=abc&status=open
29693185
// After extracting all known query parameters, any remaining keys are
29703186
// treated as implicit field-level equality filters merged into `where`.
2971-
const knownParams = new Set([
2972-
'top', 'limit', 'offset',
2973-
'orderBy',
2974-
'fields',
2975-
'where',
2976-
'expand',
2977-
'distinct', 'count',
2978-
'aggregations', 'groupBy',
2979-
'search', 'searchFields', 'context', 'cursor',
2980-
]);
3187+
// Every one of them is a verified field name by this point.
3188+
//
3189+
// The `!options.where` guard is #4134's deliberate scope edge: when an
3190+
// explicit filter won, a leftover REAL field name is still dropped
3191+
// silently (it rides on to `engine.find` as a stray AST key no driver
3192+
// reads). Same disease, opposite direction — over-returning rather than
3193+
// zeroing — and fixing it means choosing a semantics (AND-merge vs.
3194+
// reject the conflict), so it is filed as #4164 rather than smuggled in
3195+
// here. The UNKNOWN half is already loud: the gate above runs before
3196+
// this branch, so a bad name 400s either way.
29813197
if (!options.where) {
29823198
const implicitFilters: Record<string, unknown> = {};
2983-
for (const key of Object.keys(options)) {
2984-
if (!knownParams.has(key)) {
2985-
implicitFilters[key] = options[key];
2986-
delete options[key];
2987-
}
3199+
for (const key of leftoverParams) {
3200+
implicitFilters[key] = options[key];
3201+
delete options[key];
29883202
}
29893203
if (Object.keys(implicitFilters).length > 0) {
29903204
options.where = implicitFilters;
29913205
}
29923206
}
2993-
3207+
29943208
// Route to engine.aggregate() when the query has GROUP BY / aggregations.
29953209
// engine.find() does not do in-memory aggregation fallback, so without
29963210
// this branch a spec-shape aggregate request would silently return

0 commit comments

Comments
 (0)