Skip to content

Commit af2a095

Browse files
os-zhuangclaude
andauthored
fix(data): searchFields / groupBy / aggregations naming a missing field are rejected, not silently degraded (#4254) (#4315)
#4226 closed sort / select / expand; the same machine kept leaking on the remaining three field-naming read axes, and each failure corrupted something the closed axes never touched: search=alpha&searchFields=no_such -> 200 MORE rows than the narrowing allowed groupBy=[no_such] -> 200 [{no_such: null, n: <true count>}] sum(no_such) -> 200 0 - indistinguishable from a real zero Each is now refused at the shared normalizer (findData), so the list route, POST /data/:object/query, the export route and the runtime dispatcher give one answer instead of four. - searchFields -> 400 INVALID_FIELD. The select failure with the sign flipped outward: dropped unknown names emptied the override, which fell back to the FULL searchable set - a narrowing parameter that widened, changing which ROWS came back. Three messages (typo / real-but-unsearchable / stale searchableFields declaration), because the fixes differ. The allowed set is resolved by the same spec/data function the engine's search expansion consumes (resolveSearchFieldResolution, moved from objectql), so gate and engine cannot drift. - groupBy -> 400 INVALID_FIELD. The in-memory fallback projected an unknown column as null for every row: N groups collapsed into one null-keyed bucket carrying the true row count. - aggregations -> 400 INVALID_FIELD. sum(<typo>) folded undefined to 0; avg/min/max answered null. count with no field (or '*') stays legal. - Unreadable SHAPES on the aggregation axes -> 400 INVALID_QUERY - the catalog code that had no emitter, like INVALID_SORT before #4226. Tiering mirrors #4226 (no registry / no field map / legacy array map -> name gates skip; shape gates still apply). Engine tolerance for internal callers is untouched. @objectstack/rest stops logging INVALID_FILTER / INVALID_SORT / INVALID_QUERY rejections as unhandled errors. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 6a67d7a commit af2a095

13 files changed

Lines changed: 1325 additions & 85 deletions

File tree

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/objectql": patch
4+
"@objectstack/spec": patch
5+
"@objectstack/rest": patch
6+
---
7+
8+
fix(data): `searchFields` / `groupBy` / `aggregations` naming a field that does not exist are rejected, not silently degraded (#4254)
9+
10+
#4226 closed `sort` / `select` / `expand`; with the filter axis (#4134 / #4164 /
11+
#4181 / #4121) that made four field-naming read axes that either apply or fail.
12+
The same machine kept leaking on the remaining three, and each failure corrupted
13+
something the closed axes never touched:
14+
15+
```
16+
search=alpha&searchFields=no_such -> 200 MORE rows than the narrowing allowed
17+
groupBy=[no_such] -> 200 [{no_such: null, n: <true count>}] N groups collapsed into 1
18+
sum(no_such) -> 200 0 — indistinguishable from a real zero
19+
```
20+
21+
Each is now refused at the shared normalizer, so `GET /data/:object`,
22+
`POST /data/:object/query`, the export route and the runtime dispatcher give
23+
one answer instead of four.
24+
25+
- **`searchFields``400 INVALID_FIELD`.** The `select` failure with the sign
26+
flipped outward: the engine dropped unknown names and, when that emptied the
27+
override, fell back to the FULL searchable set — so a parameter that exists
28+
only to narrow a search widened it, and it changed which ROWS came back, not
29+
just which columns. Its only in-framework caller is `GET /data/:object/export`
30+
— the route whose `search` support just shipped so exports would stop
31+
downloading "the unsearched superset … in a file that looks authoritative";
32+
a typo'd `searchFields` did exactly that, one parameter over. Three causes,
33+
three messages, because the fixes differ (the split #4226 drew on expand): a
34+
name that is no field is a request typo; a REAL field outside the searchable
35+
set needs the object changed (its message names the declared
36+
`searchableFields` or the auto-default's type rule, whichever applies); and
37+
a `searchableFields` entry that names no field is a STALE DECLARATION — a
38+
bug on the object, called out as such because clients (objectui's list
39+
search) echo the declaration verbatim. The allowed set is resolved by the
40+
same `@objectstack/spec/data` function the engine's search expansion
41+
consumes (`resolveSearchFieldResolution`, moved from objectql), so the gate
42+
cannot drift from what search actually scans.
43+
- **`groupBy``400 INVALID_FIELD`.** The in-memory aggregation path projects
44+
an unknown column as `null` for every row, so all rows landed in ONE bucket
45+
whose count is the true row count — structurally perfect, identical to "this
46+
column really holds a single value". A chart draws one bar; nothing says the
47+
grouping never ran. Native SQL aggregation errors on the same input, so which
48+
backend a deployment sits on decided the answer — the "two routes, opposite
49+
answers" split, one axis over.
50+
- **`aggregations``400 INVALID_FIELD`.** `sum(<typo>)` folded a column of
51+
`undefined` to `0` — the exact number an empty quarter produces, in reports
52+
whose whole job is to be believed (`avg`/`min`/`max` answered `null` the same
53+
way). `count` with no `field` (or the `'*'` sentinel) is the one legitimate
54+
field-less form and passes.
55+
- **Unreadable SHAPES on the aggregation axes → `400 INVALID_QUERY`** — the
56+
standard-catalog code that had no emitter since it was written, like
57+
`INVALID_SORT` before #4226. A string `groupBy`, an entry naming no field, a
58+
function or `dateGranularity` outside the spec enums, a missing `alias`: each
59+
slipped past the `Array.isArray` routing guard (rows returned UNGROUPED) or
60+
computed a silent placeholder (`null` results, a column keyed `"undefined"`,
61+
one bucket per raw value under an unknown granularity).
62+
63+
Tiering is unchanged from #4226: registry + field map present → authoritative;
64+
no registry / no field map / legacy array field map → the NAME gates skip (shape
65+
gates still apply — they need no schema). The engine's own tolerance is
66+
untouched: internal callers reaching `engine.find()` / `engine.aggregate()`
67+
directly are unaffected. `@objectstack/rest` also stops logging
68+
`INVALID_FILTER` / `INVALID_SORT` / `INVALID_QUERY` rejections as
69+
"[REST] Unhandled error" — they are client mistakes the response already
70+
explains, as `INVALID_FIELD` always was.
71+
72+
Requests that name real fields are unaffected.

content/docs/api/data-api.mdx

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ Query records with filtering, sorting, selection, and pagination.
2424
| `top` | query | Max records to return. No default — omitting it returns all matching records. |
2525
| `skip` | query | Offset |
2626
| `expand` | query | Comma-separated list of relations to eager-load. Must name a reference field (`lookup` / `master_detail` / `user` / `tree`) — otherwise `400 INVALID_FIELD`. |
27-
| `search` | query | Full-text search query |
27+
| `search` | query | Full-text search term, scanned case-insensitively across the object's searchable fields (its declared `searchableFields`, or a text-like auto-default when none are declared). |
28+
| `searchFields` | query | Comma-separated subset of the searchable fields for `search` to scan — narrows the scan, never widens it. A name outside the searchable set is `400 INVALID_FIELD`. |
2829

2930
> **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.
3031
@@ -116,6 +117,46 @@ a direction that is neither `asc` nor `desc`) is `400 INVALID_SORT`.
116117
`GET /data/:object/:id` applies the same `select` and `expand` rules, so the
117118
list and single-record routes cannot disagree about one field map.
118119

120+
#### Neither is a search narrowing, a grouping, or an aggregation
121+
122+
The last three field-naming axes follow the same rule. Each of these used to
123+
answer `200` with something that looked exactly like a served query — and each
124+
corrupts something the earlier axes do not:
125+
126+
| Request | Result |
127+
|:---|:---|
128+
| `?search=alpha&searchFields=title` | scans only `title` |
129+
| `?search=alpha&searchFields=no_such_field` | `400 INVALID_FIELD` |
130+
| `?search=alpha&searchFields=amount` | `400 INVALID_FIELD` — real field, but not searchable |
131+
| `groupBy: ["status"]` | one bucket per status value |
132+
| `groupBy: ["no_such_field"]` | `400 INVALID_FIELD` |
133+
| `aggregations: [{function:"sum", field:"amount", alias:"total"}]` | the real total |
134+
| `aggregations: [{function:"sum", field:"no_such_field", alias:"total"}]` | `400 INVALID_FIELD` |
135+
| `aggregations: [{function:"count", alias:"n"}]` | `count(*)` — the one legitimate field-less form |
136+
137+
- **`searchFields`** — the only parameter whose failure changed which **rows**
138+
came back. An unknown name used to be dropped, and an override left empty
139+
fell back to scanning *every* searchable column: a parameter that exists
140+
only to **narrow** a search failed by **widening** it. Three causes get
141+
three messages, because the fixes differ: a name that is no field (a typo in
142+
the request), a real field outside the searchable set (declare it in
143+
`searchableFields`), and a `searchableFields` entry that names no field (a
144+
stale declaration — the bug is on the object, and clients that echo the
145+
declaration verbatim are told so).
146+
- **`groupBy`** — an unknown column projected `null` for every row, so all
147+
rows fell into **one bucket** whose count is the true row count:
148+
structurally perfect, indistinguishable from a column that really holds a
149+
single value. A chart draws one bar and nothing says the grouping never ran.
150+
- **`aggregations`**`sum` over an unknown column folded blanks to **`0`**,
151+
the exact number a genuinely empty quarter produces (`avg`/`min`/`max`
152+
answered `null` the same way), in reports whose whole job is to be believed.
153+
154+
A `groupBy` / `aggregations` value the spec cannot read at all — a bare string
155+
instead of an array, an entry that names no field, a function or date
156+
granularity outside the spec's enums, a missing `alias` — is
157+
`400 INVALID_QUERY`: those shapes were silently ignored, returning **ungrouped
158+
raw rows** with nothing to say the aggregation never happened.
159+
119160
**Response**:
120161
```json
121162
{

content/docs/api/error-catalog.mdx

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ ObjectStack uses a structured error system with **9 error categories** and **53
7474
**Cause:** A field name in the request does not exist on the target object. On a
7575
list read this also covers an unreserved query parameter — `GET /data/:object`
7676
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.
77+
records and is rejected rather than answered with an empty page — plus every
78+
other read axis that names a field: `select`, `expand` (a real field that holds
79+
no reference gets its own message), `searchFields` (a real field outside the
80+
searchable set gets its own message), `groupBy`, and `aggregations[].field`.
7881
**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
7982
*parameter* rather than a field, use the real one — page size is `top` / `$top`
8083
/ `limit`, not `pageSize` / `perPage`; the response's `error` names the
@@ -117,8 +120,20 @@ substitute. See the [Data API](/docs/api/data-api).
117120
**Retry:** `no_retry`
118121

119122
### `INVALID_QUERY`
120-
**Cause:** The query structure is malformed.
121-
**Fix:** Check the query syntax against the [Query Cheat Sheet](/docs/data-modeling/queries).
123+
**Cause:** A `groupBy` / `aggregations` value the spec cannot read: a bare
124+
string where an array belongs, an entry that names no field, an aggregation
125+
function or `dateGranularity` outside the spec's enums, or a missing `alias`.
126+
(Field names that simply don't exist on the object are `INVALID_FIELD`
127+
instead.)
128+
**Fix:** Check the aggregation shapes against the
129+
[Query Cheat Sheet](/docs/data-modeling/queries) — e.g.
130+
`{ "groupBy": ["status"], "aggregations": [{ "function": "sum", "field":
131+
"amount", "alias": "total" }] }`. `count` is the only function that may omit
132+
`field`.
133+
**Why it is an error and not an empty result:** every one of these shapes used
134+
to be silently ignored or mis-read — rows came back **ungrouped**, or an
135+
unknown function computed `null` — in a response indistinguishable from a
136+
served aggregation.
122137
**Retry:** `no_retry`
123138

124139
### `INVALID_FILTER`

content/docs/data-modeling/queries.mdx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,19 @@ three (only `count`/`sum`/`avg`/`min`/`max` are mapped), and the in-memory drive
364364
silently returns `null` for them. Avoid these three on SQL- or memory-backed objects.
365365
</Callout>
366366

367+
<Callout type="info">
368+
Over the REST/protocol ingress, `groupBy` and `aggregations` are validated
369+
before they reach any driver. A field the object does not have is
370+
`400 INVALID_FIELD` — the in-memory fallback used to collapse an unknown
371+
`groupBy` column into one `null`-keyed bucket, and to answer `sum(<typo>)`
372+
with `0`. A value the spec cannot read (a non-array, an entry naming no
373+
field, a function or `dateGranularity` outside the enums above, a missing
374+
`alias`) is `400 INVALID_QUERY` — those shapes used to be ignored, returning
375+
ungrouped raw rows. `count` with no `field` (or `field: "*"`) is the one
376+
legitimate field-less form and passes. Internal callers reaching
377+
`engine.aggregate()` directly are unaffected.
378+
</Callout>
379+
367380
### Aggregation Example
368381

369382
```typescript
@@ -449,6 +462,17 @@ Only `query` and `fields` are implemented. The engine expands `search` into a dr
449462
`operator`.
450463
</Callout>
451464

465+
<Callout type="info">
466+
`fields` (and its query-parameter spelling, `?searchFields=` / `?$searchFields=`) can only
467+
**narrow** the scan within the server-resolved searchable set — the object's declared
468+
`searchableFields`, or a text-like auto-default when none are declared. Over the
469+
REST/protocol ingress a name outside that set is `400 INVALID_FIELD`, with distinct
470+
messages for a field that does not exist and a real field that is not searchable. The
471+
engine used to drop unknown names and fall back to the full searchable set — a parameter
472+
that exists to narrow a search, silently widening it. Internal callers reaching
473+
`engine.find()` directly are unaffected.
474+
</Callout>
475+
452476
### Pinyin recall (Chinese deployments)
453477

454478
When pinyin search is enabled (`OS_SEARCH_PINYIN_ENABLED` — auto-on when the stack's

content/docs/protocol/objectql/query-syntax.mdx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,18 @@ FROM opportunity
616616
GROUP BY stage
617617
```
618618

619+
<Callout type="info">
620+
Over the REST/protocol ingress, both axes are validated before any driver
621+
runs: a `groupBy` entry or `aggregations[].field` naming a field the object
622+
does not have is `400 INVALID_FIELD` (the in-memory fallback used to collapse
623+
every row into one `null`-keyed bucket, and to answer `sum(<typo>)` with `0`),
624+
and a shape the spec cannot read — a non-array, an entry naming no field, an
625+
unknown function or `dateGranularity`, a missing `alias` — is
626+
`400 INVALID_QUERY`. `count` with no `field` (or `field: '*'`) is the one
627+
legitimate field-less form. Internal callers reaching `engine.aggregate()`
628+
directly are unaffected.
629+
</Callout>
630+
619631
### Aggregation Functions
620632

621633
```typescript
@@ -753,7 +765,11 @@ const query: QueryAST = {
753765
Field resolution is server-side and never client-trusted: `search.fields` is
754766
**intersected** with the object's declared `searchableFields` (or, absent those, an
755767
auto-default of the name field plus short-text/enum fields), so naming a field outside
756-
that set does not widen the search. Multiple whitespace-separated terms are AND-ed and
768+
that set can never widen the search — and over the REST/protocol ingress it is
769+
`400 INVALID_FIELD` outright (#4254), because the engine-side intersection alone used to
770+
drop the unknown name and fall back to scanning the full searchable set. Internal
771+
callers reaching `engine.find()` directly keep the tolerant intersection.
772+
Multiple whitespace-separated terms are AND-ed and
757773
fields are OR-ed. Case sensitivity is the **driver's**, not the expansion's: the
758774
expansion emits a plain `$contains`, which `SqlDriver` compiles to a parameterised
759775
`LIKE '%…%'` with no case folding — so the dialect's own `LIKE`/collation rules decide —

0 commit comments

Comments
 (0)