Skip to content

Commit 7cdbcbb

Browse files
os-helpclaude
andauthored
fix(plugin-hono-server): surface repeated query parameters as arrays (#6878) (#7396)
Flip both of the Hono adapter's IHttpRequest.query construction sites — the route-handler seam and the use() middleware seam — from c.req.query() (first value per key) to c.req.queries() through one readQuery(c) helper that normalises by length: a repeated key becomes an array, a single-valued key stays a plain string. This is #6878 route 2, adopted by the cli-lane seat on 2026-08-10. It removes the divergence PR #6941 recorded: the reference NodeHttpServer already kept arrays, so one request had two answers depending on which server booted, and a handler could not refuse an ambiguity the transport had already collapsed. The normalisation is load-bearing: c.req.queries() returns an array for EVERY key, single-valued ones included, so a bare swap would have turned every existing single-value read point on the production adapter into an array. Also collapses #6941's divergence-recording conformance case into the single expected shape, per that file's own header instructions, and adds a middleware-seam case so a half-applied two-site change cannot pass. Claude-Session: https://claude.ai/code/session_0158ZQo7LiHSxGWpYKuPq1wu Co-authored-by: Claude <noreply@anthropic.com>
1 parent ef678d0 commit 7cdbcbb

3 files changed

Lines changed: 258 additions & 96 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/plugin-hono-server": patch
3+
"@objectstack/http-conformance": patch
4+
---
5+
6+
fix(plugin-hono-server): surface a repeated query parameter as an array, matching the platform convention (#6878)
7+
8+
**Behaviour change, not a refactor.** On the Hono server, a repeated query
9+
parameter — `?version=1.0.0&version=2.0.0` — used to reach your handler as the
10+
single string `'1.0.0'`. It now reaches it as `['1.0.0', '2.0.0']`. A
11+
single-valued key is unchanged: still a plain string.
12+
13+
This is the ruled intent of #6878 (route 2, cli-lane seat ruling of
14+
2026-08-10), not an incidental cleanup.
15+
16+
**Why the old behaviour was a problem.** The platform ships two `IHttpServer`
17+
implementations, and they answered the same request differently. The reference
18+
`NodeHttpServer` reads `url.searchParams.getAll(key)` and keeps the array; the
19+
Hono adapter read `c.req.query()`, which returns only the first value per key.
20+
Both satisfied the declared contract — `IHttpRequest.query` is
21+
`Record< string, string | string[] >` — so neither had a bug, yet the
22+
platform's answer to "what is a repeated parameter?" depended on which server
23+
had booted.
24+
25+
The consequence was not cosmetic. A handler cannot refuse an ambiguity it
26+
cannot see: #6307 found `DELETE /api/v1/packages/:id` silently narrowing a
27+
destructive operation's scope from a repeated `version`, and its fix (refuse
28+
repetition with a `400`) was unreachable on the Hono server because the
29+
transport had already collapsed the duplicate. Duplicates now reach the
30+
handler on both servers, where the rest-side gates landed in #6877 (PR #7324
31+
63 single-valued parameter slots) and #7321 (PR #7386) refuse them explicitly.
32+
33+
**Both construction sites moved.** The adapter builds `IHttpRequest.query` at
34+
the route-handler seam *and* inside the `use()` middleware seam; both now go
35+
through one `readQuery(c)` helper, so middleware and handlers agree.
36+
37+
⚠️ **If you read query parameters off the Hono server, check your assumptions.**
38+
A read point that assumed a string will now receive an array when — and only
39+
when — a client repeats that parameter. `String(req.query.x)` yields `"a,b"`
40+
and `Number(req.query.x)` yields `NaN` in that case. Handle the array, or
41+
refuse the repetition explicitly; do not reach back for the first value, which
42+
is the silent-wrong-answer shape #6878 set out to remove. The repo's own read
43+
points were swept and gated before this landed.
44+
45+
Nothing in `packages/spec` changed: the declared union already permitted
46+
arrays. What changed is the platform's answer, from "depends on the server" to
47+
one answer.
48+
49+
`@objectstack/http-conformance` gets the matching test tightening. Its
50+
cross-adapter case, added under #6878 route 1 (PR #6941) to *record* the
51+
divergence, is collapsed into the single expected shape exactly as that file's
52+
own header instructed — plus a new middleware-seam case, so a half-applied
53+
change to only one of the adapter's two construction sites cannot pass. The
54+
single-value control case that catches an un-normalised `c.req.queries()`
55+
(which returns an array for every key, single-valued ones included) stays.

packages/plugins/plugin-hono-server/src/adapter.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,44 @@ function readRouteParams(c: any): Record<string, string> {
170170
}
171171
}
172172

173+
/**
174+
* The request's query parameters, with a REPEATED key surfacing as an array
175+
* and a single-valued key as a plain string (#6878, route 2).
176+
*
177+
* `c.req.query()` — what both construction sites used before — returns only the
178+
* FIRST value per key, so `?version=1.0.0&version=2.0.0` reached a handler as
179+
* `'1.0.0'` and the ambiguity was gone before anyone could refuse it. The
180+
* reference `NodeHttpServer` in `packages/qa/http-conformance` never collapsed:
181+
* it reads `url.searchParams.getAll(key)` and keeps the array. One request
182+
* therefore had two answers depending on which server booted, which is what
183+
* #6878 measured and what the cli-lane seat ruled (2026-08-10) to resolve in
184+
* this direction — the handler must be able to SEE the ambiguity in order to
185+
* reject it, per #6307's landed `readSingleQueryValue` direction.
186+
*
187+
* ⚠️ The normalisation is NOT optional. `c.req.queries()` returns an array for
188+
* EVERY key, single-valued ones included (measured on hono@4.12.x:
189+
* `{ version: ['1.0.0','2.0.0'], single: ['9'] }`), so a bare swap would have
190+
* silently turned every existing single-value read point on this adapter into
191+
* an array. PR #6941 left a control case in
192+
* `packages/qa/http-conformance/src/query-multiplicity.conformance.test.ts`
193+
* that fails on exactly that mistake — keep it green.
194+
*
195+
* Built with `Object.fromEntries` rather than dynamic property writes
196+
* (`obj[key] = …`): the keys come straight off the wire, and `?__proto__=…`
197+
* through a dynamic write is remote property injection. `fromEntries` creates
198+
* own data properties only. Same reasoning, same shape, as the reference
199+
* adapter's query block.
200+
*/
201+
function readQuery(c: any): Record<string, string | string[]> {
202+
const all: Record<string, string[]> = c.req.queries() ?? {};
203+
return Object.fromEntries(
204+
Object.entries(all).map(([key, values]) => [
205+
key,
206+
values.length > 1 ? values : values[0],
207+
]),
208+
) as Record<string, string | string[]>;
209+
}
210+
173211
/**
174212
* Hono Implementation of IHttpServer
175213
*/
@@ -296,7 +334,7 @@ export class HonoHttpServer implements IHttpServer {
296334

297335
const req = {
298336
params: readRouteParams(c),
299-
query: c.req.query(),
337+
query: readQuery(c),
300338
body,
301339
headers: rawHeaders,
302340
method: c.req.method,
@@ -740,7 +778,7 @@ export class HonoHttpServer implements IHttpServer {
740778
const headers = c.req.header() as Record<string, string>;
741779
const req = {
742780
params: {},
743-
query: c.req.query(),
781+
query: readQuery(c),
744782
// Deliberately absent — see the `use()` contract above.
745783
body: undefined,
746784
headers,

0 commit comments

Comments
 (0)