Skip to content

Commit e98c9d3

Browse files
hotlongclaude
andauthored
fix(rest): refuse a repeated ?filter= as a repetition, not as a malformed filter (#7390) (#8004)
Since #6878 route 2 (PR #7396) the Hono adapter surfaces repeated query parameters as arrays, so a repeated `?filter=` on `GET /data/:object` now reaches the shared list-query normalizer. That normalizer cannot judge the slot: a filter AST IS an array (`["status","=","open"]`), so #7386's arity gate had to leave it alone, and the two ingresses it serves — this querystring and `POST /data/:object/query`'s arbitrary-JSON body — are byte-identical there. Two shapes came out of that, both live: ?filter={"a":1}&filter={"b":2} -> 400, diagnosed as MALFORMED ?filter=status&filter=%3D&filter=open -> 200, applying a filter nobody wrote The first told a caller whose filters were both well-formed to check their AST syntax. The second spelled a valid AST by accident and succeeded. The arity judgement now happens at the REST querystring parse, the only layer that knows it is looking at a querystring — there an array on the filter slot is a repeated parameter and can be nothing else, so the normalizer stays free of the heuristic #4181 and #4121 removed. All four wire spellings of the one slot are covered (`filter`/`where` derived from the spec's own RPC_QUERY_ALIAS_SLOTS, `filters`/`$filter` wire-only). Refused, never resolved (maintainer ruling 2026-08-11): last-wins and AND-merge each silently serve one of two intents the caller expressed. The gate throws rather than responding, so the answer keeps the flat `mapDataError` envelope this route's other filter refusals already use — one slot, one wire code (`INVALID_FILTER`, already standard-catalog), one body shape, whether the filter was unreadable or sent twice. Unaffected: a single `?filter=` in both accepted forms, the POST body face, the genuinely multi-valued parameters, and a one-element array from a repeat-preserving adapter (one occurrence, unwrapped). Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V Co-authored-by: Claude <noreply@anthropic.com>
1 parent fce8e49 commit e98c9d3

4 files changed

Lines changed: 714 additions & 4 deletions

File tree

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): a repeated `?filter=` on `GET /data/:object` is refused as a repetition, not misdiagnosed as a malformed filter (#7390)
6+
7+
**This is a behaviour change on a live surface.** A request that previously
8+
answered `200` now answers `400`, and a request that already answered `400` now
9+
carries a different message. Both changes make the response describe what the
10+
caller actually did.
11+
12+
Repeating a query parameter used to be invisible: the production Hono adapter
13+
collapsed repeats to the first value before any handler ran. Since #6878 route 2
14+
(PR #7396) it surfaces them as arrays, so a repeated `?filter=` now reaches the
15+
shared list-query normalizer — and that normalizer structurally cannot tell what
16+
it is looking at. A filter AST **is** an array (`["status","=","open"]`), so the
17+
arity gate #7386 added to every other query slot had to leave this one alone: on
18+
the filter slot, an array is the ordinary shape of a legitimate body-form filter
19+
sent to `POST /data/:object/query`.
20+
21+
Two shapes came out of that, both live:
22+
23+
| request | before | now |
24+
| :--- | :--- | :--- |
25+
| `?filter={"a":1}&filter={"b":2}` | `400 INVALID_FILTER`, diagnosed as a **malformed** filter | `400 INVALID_FILTER`, diagnosed as a **repetition** |
26+
| `?filter=status&filter=%3D&filter=open` | **`200`**, applying `{status:"open"}` | `400 INVALID_FILTER` |
27+
28+
The first was the common one, and its message was actively misleading: both
29+
filters the caller sent were well-formed, the response told them to check their
30+
AST syntax, and the operator vocabulary it listed could not help. The second is
31+
contrived to write by hand but is the sharper defect — three occurrences of one
32+
parameter happened to spell a valid AST, so the request succeeded while applying
33+
a filter nobody expressed.
34+
35+
The refusal now names the condition: `Repeated "filter" query parameter — send
36+
exactly one.` A repeated filter is **not** merged and **not** resolved by
37+
precedence — either would silently serve one of two intents the caller actually
38+
expressed, which is the authoring trap this refusal exists to close.
39+
40+
The judgement is made at the REST querystring parse rather than in the shared
41+
normalizer, because the querystring layer is the only one that knows it is
42+
looking at a querystring: there, an array on the filter slot is a repeated
43+
parameter and can be nothing else. All four wire spellings of the one slot
44+
(`filter`, `where`, `filters`, `$filter`) are covered.
45+
46+
**Unaffected:**
47+
48+
- A **single** `?filter=` in either accepted form — the JSON object
49+
(`?filter={"status":"open"}`) and the bare AST
50+
(`?filter=["status","=","open"]`).
51+
- `POST /data/:object/query` — the body face legitimately sends an array, and is
52+
untouched.
53+
- Genuinely multi-valued query parameters (`$select`, `$expand`,
54+
`$searchFields`), which keep their array arm.
55+
- A one-element array from a repeat-preserving adapter, which is one occurrence
56+
and is unwrapped rather than refused — this also stops it being read as a
57+
malformed AST.
58+
59+
No spec change: `INVALID_FILTER` is already a standard-catalog code, and the
60+
accepted wire forms of `filter` are unchanged.

packages/rest/src/query-multiplicity.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

3+
import { RPC_QUERY_ALIAS_SLOTS } from '@objectstack/spec/data';
4+
35
/**
46
* Query-parameter MULTIPLICITY, for every REST handler in this package (#6877).
57
*
@@ -145,3 +147,110 @@ export function refuseRepeatedQueryParams(
145147
}
146148
return false;
147149
}
150+
151+
/**
152+
* Every WIRE spelling of the ONE filter slot, as `GET /data/:object` receives
153+
* it (#7390).
154+
*
155+
* The canonical key and its schema-declared alias are read off the spec's own
156+
* table — `RPC_QUERY_ALIAS_SLOTS`, "the ONE place the alias to canonical
157+
* mapping is declared" — so a spelling added there reaches this gate without
158+
* anybody remembering to copy it.
159+
*
160+
* `filters` and `$filter` are named here instead, because they are wire-only:
161+
* no schema declares them, and `metadata-protocol` extends that same spec
162+
* table with them for exactly that reason. Deriving them was not an option —
163+
* `@objectstack/metadata-protocol` is a dev-only dependency of this package,
164+
* so no runtime import of its table exists. Gating fewer than all four would
165+
* leave three quarters of one slot misdiagnosed, which is the defect, not a
166+
* narrower version of the fix. `filterSlotSpellingsAreComplete` in
167+
* `rest-server-repeated-filter-param.test.ts` pins the composition so a spec
168+
* table that loses `where` goes red here rather than silently ungating a
169+
* spelling.
170+
*/
171+
export const FILTER_SLOT_QUERY_PARAMS: readonly string[] = (() => {
172+
const slot = RPC_QUERY_ALIAS_SLOTS.find((s) => s.canonical === 'where');
173+
return [...(slot ? [slot.canonical, ...slot.aliases] : []), 'filters', '$filter'];
174+
})();
175+
176+
/**
177+
* The one refusal message for a repeated filter parameter.
178+
*
179+
* It names REPETITION, and that is the whole point of #7390 rather than a
180+
* wording preference. Until this gate existed the same request was answered by
181+
* `malformedFilterArrayError` in the normalizer — a 400 whose text told the
182+
* caller their filter was *malformed*, listing the AST operator vocabulary,
183+
* when every filter they sent was well-formed and the mistake was sending two.
184+
* A caller reading that message re-checks their syntax, which is the one thing
185+
* that cannot help them.
186+
*
187+
* It also does not offer a resolution, because there is none to offer
188+
* (maintainer ruling, 2026-08-11): last-wins and AND-merge were both rejected
189+
* as silent selection among duplicates.
190+
*/
191+
export function repeatedFilterParamMessage(name: string, count: number): string {
192+
return `Repeated "${name}" query parameter — send exactly one. It was supplied ${count} times. `
193+
+ 'A repeated filter is neither merged nor resolved by precedence: either would apply a '
194+
+ 'filter you did not express.';
195+
}
196+
197+
/**
198+
* Refuse a repeated filter parameter on a QUERYSTRING ingress (#7390).
199+
*
200+
* ## Why this rule cannot live in the shared normalizer
201+
*
202+
* `metadata-protocol`'s list-query normalizer serves two ingresses through one
203+
* door — `GET /data/:object`, where a repeat arrives as `string[]`, and
204+
* `POST /data/:object/query`, whose body is arbitrary JSON — and a filter AST
205+
* *is* an array (`['status','=','open']`). So the two are byte-identical
206+
* there, which is precisely why #7386's arity gate had to leave this slot
207+
* alone ({@link https://github.com/objectstack-ai/objectstack/issues/7390}).
208+
* On a querystring the ambiguity does not exist: an array on the filter slot
209+
* is a repeated parameter and can be nothing else. This layer is the only one
210+
* that knows it is looking at a querystring, so the judgement is made here and
211+
* the normalizer stays free of the heuristic (`an array of strings each
212+
* parseable as JSON is probably a repetition`) that #4181 and #4121 spent
213+
* effort removing.
214+
*
215+
* ## Why it THROWS instead of responding
216+
*
217+
* Its sibling {@link refuseRepeatedQueryParams} writes the ADR-0112 NESTED
218+
* body itself, which is right for the `/meta` family it guards. The data
219+
* routes speak the FLAT `mapDataError` envelope (`{ error, code, object }`),
220+
* and that is the envelope this route's OTHER filter refusals already arrive
221+
* in — `unusableFilterError` and `malformedFilterArrayError` both throw
222+
* `400` / `INVALID_FILTER` and are shaped by the handler's own catch. So this
223+
* gate throws the same shape from inside the same `try`: one slot, one wire
224+
* code, one body shape, whether the filter was unreadable or sent twice.
225+
* Responding here instead would author a second dialect for one condition.
226+
*
227+
* `INVALID_FILTER` is a STANDARD-catalog code (`spec/src/api/errors.zod.ts`),
228+
* not a new one — nothing in `packages/spec` moves for this.
229+
*
230+
* A one-element array is one occurrence encoded by an adapter, and is unwrapped
231+
* rather than refused — the same count-not-shape rule this module's header
232+
* states, and the reason a `['{"a":1}']` from a repeat-preserving adapter stops
233+
* being read as a malformed AST too.
234+
*
235+
* @param query the handler's `req.query` (`any`-shaped; `rest-server.ts` types
236+
* its handlers that way). Non-object values are left alone.
237+
* @throws a `400` / `INVALID_FILTER` error when a filter spelling was supplied
238+
* more than once.
239+
*/
240+
export function assertFilterParamSuppliedOnce(query: unknown): void {
241+
if (!query || typeof query !== 'object') return;
242+
const bag = query as Record<string, unknown>;
243+
for (const name of FILTER_SLOT_QUERY_PARAMS) {
244+
const raw = bag[name];
245+
if (!Array.isArray(raw)) continue;
246+
const read = readSingleQueryValue(raw as string[]);
247+
if (!read.ok) {
248+
const err: any = new Error(repeatedFilterParamMessage(name, read.count));
249+
err.status = 400;
250+
err.code = 'INVALID_FILTER';
251+
err.param = name;
252+
throw err;
253+
}
254+
bag[name] = read.value as string;
255+
}
256+
}

0 commit comments

Comments
 (0)