ReviewOS

home-lang/home

fix(router): give percent-decoded matches stable owned path storage

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

Parent: #66.

Home's URLPath.parse percent-decodes into a shared thread-local buffer. FileSystemRouter.match returns route pathname and parameter slices into that buffer, so a later encoded match overwrites data retained by an earlier MatchedRoute. The same path flow also has an explicit leak TODO because the original path allocation is never owned by the returned route.

Bun's live implementation gives each decoded URLPath heap-stable storage, transfers that storage into the MatchedRoute, and keeps the ordinary input allocation there when no decode occurs.

Port an allocator-backed URLPath parse wrapper that rebases every derived slice into per-call storage. Move the final path backing into MatchedRoute and release it during finalization. Preserve transient URLPath.parse for existing synchronous router call sites.

Acceptance:

  • Two consecutive percent-encoded matches retain independent pathname and parameter values.
  • Plain paths and %25 values preserve their existing parse semantics.
  • No path allocation leaks on successful, unmatched, or parse-error paths.
  • The unchanged filesystem-router test passes at its original deadline against a fresh runtime.
9 days ago

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

URLPath.parseOwned now copies percent-decoded output into per-call allocator storage and rebases pathname, path, first-segment, extension, and query slices. FileSystemRouter.match transfers either that decoded allocation or the ordinary input allocation into MatchedRoute; finalization releases the backing allocation, and unmatched/parse-error/OOM paths release locally owned storage. Allocation failure now throws the JSC out-of-memory error, and a failed MatchedRoute allocation releases its cloned parameter list.

All 10 direct URLPath tests pass, including stability across two consecutive encoded parses and %25 decoding exactly once. zig fmt --check and git diff --check pass. Supervisor peak was 161 MB.

The unchanged full filesystem-router test against a fresh linked runtime remains pending because the debug link exceeded the conservative memory ceilings documented on #729. This issue stays open until that acceptance run is available.

9 days ago

Verified complete on current origin/main at 2e2b71edde.

Implementation is in a1387e3a4. A fresh unchanged filesystem_router.test.ts run passes the production ownership controls for:

  • consecutive encoded matches retaining independent pathname and param values;
  • unencoded paths and %25 decoding;
  • successful-match lifetime under GC pressure;
  • clean invalid-route failure without use-after-free;
  • MatchedRoute parameter leak coverage.

The complete file finishes 22 pass / 1 upstream skip / 0 fail, 1,500 assertions, at the original five-second test deadline. No source/test edit, retry, or deadline increase was used. Closing as completed; parent #66 remains open for full parity.

Sign in to comment on this issue.