Skip to content

Commit bf478e1

Browse files
authored
fix(data): sort / select / expand naming a missing field are rejected, not dropped (#4226) (#4240)
The list path has four axes on which a caller names a field. `filter` was closed over #4134 / #4164 / #4181 / #4121; the other three still answered 200 when the name was wrong: sort=no_such_field -> 200 CAEBD identical to "no sort at all" select=no_such_field -> 200 <every field> asked for one column, got all expand=no_such_rel -> 200 <no such key> no relation, no complaint Each is now refused in the shared normalizer, so the list route, the single-record route, POST /data/:object/query, the export route and the runtime dispatcher give one answer instead of five. - sort -> 400 INVALID_SORT. The row set is unchanged, so this is not #4181's "returned everything" — but sort + top is how a caller asks for "the latest N", and a dropped sort makes that an arbitrary N. INVALID_SORT had sat in the standard catalog with no emitter since it was written. - select -> 400 INVALID_FIELD. engine.find() drops unknown columns and then falls back to `*` when that empties the projection, so ?select=<typo> asked for ONE column and received EVERY column: a parameter that exists to return less, failing by returning more. - expand -> 400 INVALID_FIELD, with distinct messages for "no such field", "field holds no reference" and "reference field declares no target". Two sort spellings that were silently never applied now are: the client SDK's declared `orderBy: string[]`, and the `{field: direction}` map that the export route, GET /data/import/jobs and objectui's calendar emit. $expand of a `tree` field also works now — the expand loop and the new gate both read the shared REFERENCE_VALUE_TYPES set. The engine's own tolerance is untouched: it guards internal callers that never pass through this ingress.
1 parent 7bf5349 commit bf478e1

10 files changed

Lines changed: 1079 additions & 46 deletions

File tree

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/objectql": patch
4+
---
5+
6+
fix(data): `sort` / `select` / `expand` naming a field that does not exist are rejected, not silently dropped (#4226)
7+
8+
The list path has four axes on which a caller names a field. `filter` was
9+
closed over #4134 / #4164 / #4181 / #4121 — a filter the server cannot apply is
10+
now a 400, never a 200 over the wrong rows. The other three still leaked, all
11+
answering `200`:
12+
13+
```
14+
sort=no_such_field -> 200 CAEBD byte-identical to "no sort at all"
15+
select=no_such_field -> 200 <every field> asked for one column, got all of them
16+
expand=no_such_rel -> 200 <no such key> no relation, no complaint
17+
```
18+
19+
Each is now refused at the shared normalizer, so `GET /data/:object`,
20+
`GET /data/:object/:id`, `POST /data/:object/query`, the export route and the
21+
runtime dispatcher give one answer instead of five.
22+
23+
- **`sort``400 INVALID_SORT`.** The row set is unchanged, so this is not
24+
#4181's "returned everything" — it is worse in one specific way: `sort` +
25+
`top` is how a caller asks for "the latest N", and a dropped sort makes that
26+
an arbitrary N that nothing in the response reveals. This is the list half of
27+
the bug #4181 fixed on the export route's `orderby`. `INVALID_SORT` had sat
28+
in the standard catalog since it was written with no emitter.
29+
- **`select``400 INVALID_FIELD`.** `engine.find()` drops unknown columns
30+
(deliberate `SELECT *` tolerance) and then falls back to `*` when that empties
31+
the projection, and the two compose into `?select=<typo>` asking for ONE
32+
column and receiving EVERY column — a parameter whose purpose is to return
33+
less, failing by returning more, against both FLS and data minimisation. The
34+
partially-unknown case (`?select=title,no_such`) is refused on the same terms:
35+
half a projection is not the one that was asked for, and the tolerant reading
36+
would have to explain why `?status=<typo>` is a 400 and `?select=<typo>` is
37+
not, on one endpoint, about one field map.
38+
- **`expand``400 INVALID_FIELD`.** The lightest of the three — same rows,
39+
same columns, the relation simply is not there — but the response cannot be
40+
told apart from "every foreign key is null", and the client renders raw ids
41+
where names belong. A name that is no field at all and a name that is a field
42+
holding no reference (`?expand=title`) get different messages, since the fixes
43+
differ.
44+
45+
**Sorts that were silently never applied now are.** Two wire spellings reached
46+
the normalizer and fell through it untouched, and every driver then declined
47+
them (`SqlDriver` guards its ORDER BY with `Array.isArray(orderBy)`): the
48+
client SDK's own declared `orderBy: string[]`, and the `{field: direction}` map
49+
that `GET /data/:object/export`, `GET /data/import/jobs` and objectui's calendar
50+
all emit. Both are now folded to `SortNode[]` — so the import-job history, which
51+
has asked for `created_at desc` since it was written and served insertion order,
52+
sorts. A sort shape that still cannot be read (a number, an entry naming no
53+
field, a direction that is neither `asc` nor `desc`) is `400 INVALID_SORT`
54+
rather than a silent no-op.
55+
56+
**`$expand` of a `tree` field works.** `REFERENCE_VALUE_TYPES` lists `tree`
57+
among the types whose value "points at another record … the related record
58+
object in expanded form", and objectui requests it, but
59+
`engine.expandRelatedRecords` tested membership with a hand-copied `!==` chain
60+
that omitted it — so a hierarchy field came back as a raw parent id. The loop
61+
now reads the shared spec set, which is also what the new expand gate validates
62+
against, so the gate cannot admit a field the engine then skips.
63+
64+
**What changes for callers:** requests naming a non-existent field in `sort`,
65+
`select` or `expand` now fail loudly instead of receiving an unsorted, widened
66+
or unexpanded response. Every axis naming real fields is unaffected. The
67+
engine's own tolerance is untouched — it guards internal callers (hooks, flows,
68+
expand sub-reads, registry-less hosts) that never pass through this ingress,
69+
the same tiering the object-existence and unknown-field gates already use.

content/docs/api/data-api.mdx

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ Query records with filtering, sorting, selection, and pagination.
1818
| Parameter | Location | Description |
1919
|:----------|:---------|:------------|
2020
| `object` | path | Object name |
21-
| `select` | query | Comma-separated field names |
21+
| `select` | query | Comma-separated field names. Every name must exist — an unknown one is `400 INVALID_FIELD`, never dropped. |
2222
| `filter` | query | Filter expression (JSON). `filters` also accepted for backward compatibility. Malformed JSON is rejected with `400 INVALID_FILTER` — never ignored. |
23-
| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`) |
23+
| `sort` | query | Sort expression (e.g. `name asc` or `-created_at`). Must name a real field — otherwise `400 INVALID_SORT`. |
2424
| `top` | query | Max records to return. No default — omitting it returns all matching records. |
2525
| `skip` | query | Offset |
26-
| `expand` | query | Comma-separated list of relations to eager-load |
26+
| `expand` | query | Comma-separated list of relations to eager-load. Must name a reference field (`lookup` / `master_detail` / `user` / `tree`) — otherwise `400 INVALID_FIELD`. |
2727
| `search` | query | Full-text search query |
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.
@@ -78,6 +78,43 @@ successful query:
7878

7979
The same rule applies to `orderby` on `GET /data/:object/export`.
8080

81+
#### Nor is a sort, a projection, or an expansion
82+
83+
`filter` is not the only parameter that names a field. `sort`, `select` and
84+
`expand` do too, and each one used to be dropped in silence when the name was
85+
wrong — three more responses that looked exactly like successful ones:
86+
87+
| Request | Result |
88+
|:---|:---|
89+
| `?sort=-created_at` | sorts |
90+
| `?sort=no_such_field` | `400 INVALID_SORT` |
91+
| `?sort={oops` / `?sort=title:desc` | `400 INVALID_SORT` — the list route spells a direction with a space (`title desc`) or a leading `-` |
92+
| `?select=id,title` | projects those two columns |
93+
| `?select=no_such_field`, `?select=title,no_such_field` | `400 INVALID_FIELD` |
94+
| `?expand=owner_id` | expands the reference |
95+
| `?expand=no_such_rel` | `400 INVALID_FIELD` — no such field |
96+
| `?expand=title` | `400 INVALID_FIELD` — real field, but it holds no reference |
97+
98+
Why each one matters, since none of them changes which rows match:
99+
100+
- **`sort`**`sort` + `top` is how you ask for "the latest N". A sort that is
101+
dropped turns that into an **arbitrary** N, and nothing in the response says so.
102+
- **`select`** — an unknown column used to be dropped, and a projection left with
103+
no known column fell back to *every* column: a parameter that exists to return
104+
**less** failed by returning **more**.
105+
- **`expand`** — an unexpanded relation is indistinguishable from one whose
106+
foreign keys are all null, so clients render raw ids where names belong.
107+
108+
Sorts accept any of these spellings, all equivalent:
109+
`?sort=-created_at`, `?$orderby=-created_at`, and — on
110+
`POST /data/:object/query` — `{"orderBy": [{"field": "created_at", "order":
111+
"desc"}]}`, `{"orderBy": ["-created_at"]}` or `{"orderBy": {"created_at":
112+
"desc"}}`. A shape that is none of these (a number, an entry naming no field,
113+
a direction that is neither `asc` nor `desc`) is `400 INVALID_SORT`.
114+
115+
`GET /data/:object/:id` applies the same `select` and `expand` rules, so the
116+
list and single-record routes cannot disagree about one field map.
117+
81118
**Response**:
82119
```json
83120
{
@@ -96,8 +133,8 @@ Get a single record by ID. Only `select` and `expand` query parameters are allow
96133
|:----------|:---------|:------------|
97134
| `object` | path | Object name |
98135
| `id` | path | Record ID |
99-
| `select` | query | Comma-separated field names to include |
100-
| `expand` | query | Comma-separated list of relations to eager-load |
136+
| `select` | query | Comma-separated field names to include. Unknown name → `400 INVALID_FIELD`. |
137+
| `expand` | query | Comma-separated list of relations to eager-load. Not a reference field → `400 INVALID_FIELD`. |
101138

102139
**Response**: `{ object: "account", id: "1", record: { ... } }`
103140

content/docs/data-modeling/queries.mdx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,14 @@ Sort results with `orderBy` (array of sort nodes):
216216
| `field` | `string` | Field name to sort by |
217217
| `order` | `'asc' \| 'desc'` | Sort direction |
218218

219+
<Callout type="info">
220+
Over the REST/protocol ingress, `orderBy` also accepts `'-created_at'`,
221+
`['-created_at']` and `{created_at: 'desc'}`, all normalized to the node array
222+
above. A sort naming a field the object does not have — or an `order` that is
223+
neither `asc` nor `desc` — is `400 INVALID_SORT` there rather than a dropped
224+
sort. Internal callers reaching `engine.find()` directly are unaffected.
225+
</Callout>
226+
219227
---
220228

221229
## Pagination
@@ -288,13 +296,20 @@ and you get every column; if it doesn't, the error is rethrown. Either way there
288296
`owner.name` key.
289297

290298
Use `expand` to pull in a relationship's fields instead.
299+
300+
Both of these are `engine.find()` behaviours, reached by internal callers. Over
301+
the REST/protocol ingress a projection column that names no field at all is
302+
`400 INVALID_FIELD` — including the case where *no* requested column is known,
303+
which used to fall back to `SELECT *` and answer a one-column request with every
304+
column.
291305
</Callout>
292306

293307
---
294308

295309
## Expand (Related Records)
296310

297-
Load related records through `lookup`, `master_detail`, and `user` fields with
311+
Load related records through the reference field types — `lookup`,
312+
`master_detail`, `user` and `tree` (`REFERENCE_VALUE_TYPES`) — with
298313
`expand`. Each key is a relationship field name; the value is a nested query that
299314
can select fields, filter, and expand further (max depth 3 — a fixed constant, not
300315
configurable).

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,14 @@ const opportunities = await engine.find('opportunity', {
415415

416416
Sorting uses the **`orderBy`** array of `SortNode` objects.
417417

418+
<Callout type="info">
419+
Over the REST/protocol ingress, `orderBy` also accepts `'-created_at'`,
420+
`['-created_at']` and `{created_at: 'desc'}` — all normalized to the
421+
`SortNode[]` above. A sort naming a field the object does not have is
422+
`400 INVALID_SORT` there, rather than being dropped. Internal callers reaching
423+
`engine.find()` directly are unaffected.
424+
</Callout>
425+
418426
### Single Field Sort
419427

420428
```typescript
@@ -449,13 +457,17 @@ const query: QueryAST = {
449457
`SqlDriver.find()` then retries **without the sort**, so the rows come back unordered
450458
rather than sorted — and no error surfaces. Denormalise the value onto the queried
451459
object (for example with a formula or rollup field) when you need to sort by it.
460+
461+
This is the one sort that still degrades silently: the REST ingress judges a
462+
dotted path on its **head segment** (`account` here, a real field), so the
463+
`INVALID_SORT` gate passes it through to the driver backstop.
452464
</Callout>
453465

454466
---
455467

456468
## 4. Relationships (Expand)
457469

458-
The `expand` property enables **recursive loading of related records** through `lookup`, `master_detail`, and `user` fields. Each key is a relationship field name; the value is a nested `QueryAST`.
470+
The `expand` property enables **recursive loading of related records** through the reference field types — `lookup`, `master_detail`, `user` and `tree` (`REFERENCE_VALUE_TYPES`). Each key is a relationship field name; the value is a nested `QueryAST`. Over the REST/protocol ingress, a key that is not one of those is `400 INVALID_FIELD` rather than a silently absent relation.
459471

460472
### Basic Expand
461473

content/docs/references/api/odata.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ const result = ODataConfig.parse(data);
205205
| **$orderby** | `string \| string[]` | optional | Sort order |
206206
| **$top** | `integer` | optional | Max results to return |
207207
| **$skip** | `integer` | optional | Results to skip |
208-
| **$expand** | `string \| string[]` | optional | Navigation properties to expand (lookup/master_detail fields) |
208+
| **$expand** | `string \| string[]` | optional | Navigation properties to expand (reference fields: lookup/master_detail/user/tree) |
209209
| **$count** | `boolean` | optional | Include total count |
210210
| **$search** | `string` | optional | Search expression |
211211
| **$format** | `Enum<'json' \| 'xml' \| 'atom'>` | optional | Response format |

0 commit comments

Comments
 (0)