ReviewOS

home-lang/home

fix(router): cap query materialization at Bun’s 2048-parameter limit

#729
Closed glennmichael123 opened this 9 days ago ·
9 days ago

Parent: #66.

Bun.FileSystemRouter materializes matched query parameters through QueryStringMap, whose iterator uses a fixed 2,048-bit visited set. Home currently collects every query parameter, so a request with more than 2,048 entries lets iteration index beyond that set and the unchanged corpus test times out or aborts. Bun's live implementation caps both counting and collection at 2,048 and bounds the visited scan.

Port the same limit into both QueryStringMap.init and initWithScanner, bound iterator scanning before reading the bitset, and preserve route-parameter precedence. Add direct coverage for plain and percent-decoded 3,000-parameter inputs. Also ensure the newly activated JSObject.putRecord bridge propagates a pending JSC exception rather than returning success.

Acceptance:

  • The unchanged filesystem_router.test.ts query-cap case completes at its original 5-second deadline.
  • Direct map tests retain exactly 2,048 entries and terminate iteration.
  • Existing query, dynamic-param, URL, and native regression coverage remains green.
  • No timeout increase, test edit, retry, or input special case.
9 days ago

Published 8b02aebac to origin/main, authored by glennmichael123 <gtorregosa@gmail.com> without coauthors. The commit references #729 and #66.

Home now applies Bun's 2,048-parameter ceiling while counting and materializing both plain and percent-decoded query strings, bounds visited-bitset access before reading it, and stops both fast and decoding paths at the same limit.

Focused verification compiled url.zig in isolation and passed all 3 selected tests. The new test checks 3,000 unique plain parameters and 3,000 percent-decoded values, retained count 2,048, first/last boundary lookup, and terminating iteration. Supervisor peak was 165 MB; zig fmt --check and git diff --check passed.

The full debug runtime build was attempted sequentially with one Zig job. It was stopped at 3,584 MB and then 4,608 MB while the host guard reported 33% headroom. The ceiling was not raised again. Therefore the unchanged filesystem_router.test.ts acceptance run against a fresh binary remains pending, and this issue stays open. The pre-change binary's baseline was 19 pass / 1 skip / 3 fail, with the query-cap case timing out at its original 5-second deadline.

9 days ago

Follow-up 18f8dab8a is published to origin/main, authored by glennmichael123 <gtorregosa@gmail.com> without coauthors. It references #729 and #66.

JSObject.putRecord now opens a TopExceptionScope, calls the native JSC record-property writer, and returns error.JSError when the native call leaves an exception pending. This mirrors the established JSObject.getIndex bridge pattern and prevents query-object initialization from returning apparent success after a property write throws.

The file passes Zig AST validation, zig fmt --check, and git diff --check. Full fresh-runtime acceptance remains pending under the memory-bound limitation recorded above, so #729 remains open.

9 days ago

Verified complete on current origin/main at 2e2b71edde.

Implementation is in 8b02aebac, with pending-exception propagation in a1ef86ed3.

Fresh acceptance evidence:

  • exact filtered home_rt build: 23/23 steps, 4/4 QueryStringMap tests;
  • direct plain and percent-decoded 3,000-entry maps retain exactly 2,048 entries and terminate iteration;
  • the unchanged filesystem_router.test.ts completes at its original five-second test deadline: 22 pass / 1 upstream skip / 0 fail, 1,500 assertions;
  • route-parameter precedence remains green in the same direct and corpus controls.

No source/test edit, retry, input special case, or deadline increase was used. Closing as completed; parent #66 remains open for full parity.

Sign in to comment on this issue.