Skip to content

test(dispatch,global,security): migrate leak-class specs to shared route helpers (PR3) - #4204

Merged
bpamiri merged 5 commits into
developfrom
peter/snapshot-routes-leaks-4194
Oct 5, 2026
Merged

bpamiri merged 5 commits into
developfrom
peter/snapshot-routes-leaks-4194

Conversation

@bpamiri

@bpamiri bpamiri commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

PR3 (final) of the staged #4194 route-state-helper migration. Builds on the shared $snapshotRoutes() / $restoreRoutes() / $clearRoutes() helpers (#4202) and the mapper-area migration (#4203), both on develop.

What

Migrates the remaining leak-class specs onto the shared helpers:

  • dispatch/findMatchingRouteMegaSpec
  • global/urlforSpec
  • security/PaginationXssSpec

Each replaces hand-rolled route save/restore with $snapshotRoutes() / $restoreRoutes() and drops its local $clearRoutes shim (now inherited from wheels.WheelsTest).

Why

These were the last specs on the partial-clear list — each restored only a subset of route state (dropping namedRoutePositions and/or staticRoutes), the #4192 leak class. With these migrated, every route-manipulating spec in the suite uses the full-state helper, so none leaks stale routes into later specs.

Verification

Local (SQLite), Lucee 7 and BoxLang, all scopes 0 fail / 0 error:

  • wheels.tests.specs.dispatch (27 bundles) — 261 pass
  • wheels.tests.specs.global (41 bundles) — 321 pass
  • wheels.tests.specs.security (36 bundles) — 403 pass
  • wheelstest.SnapshotRoutesSpec — 2 pass
  • internal.RequireRepoPathSpec (WHEELS_EXPECT_REPO=true, env test exercised) — 4 pass

Full cross-engine matrix dispatched separately.

Closes #4194

🤖 Generated with Claude Code

bpamiri and others added 3 commits October 3, 2026 06:19
…tes helpers (#4194)

Route specs that redefine the table must restore it in afterEach or they leak
stale state into later specs. routes, staticRoutes, namedRoutePositions, the
urlFor caches (application- AND request-scoped) and #4183's router index are
separate structures, so a partial restore is the #4192 bug class (and
formsSpec already restored only routes + namedRoutePositions, dropping
staticRoutes).

Add three helpers to wheels.WheelsTest:
- $snapshotRoutes() -> deep-copy snapshot of every route-derived structure
  present (#4183 fields behind StructKeyExists, so no rebase needed);
- $restoreRoutes(snapshot) -> exact restore, deleting managed keys that were
  absent at snapshot time;
- $clearRoutes() -> empty the table + invalidate every route cache (shared,
  replacing the per-spec shims).

PR1 of a staged migration: helpers + a round-trip self-test
(SnapshotRoutesSpec) + migrate the view/* specs (links, urls,
paginationHelpers, forms). dispatch/mapper/global/middleware/security/hardener
follow in area PRs. Design: shared-docs state/dev1-design/evidence/4194-*.

Updates the CLAUDE.md / consumer-ai "Test-specific gotchas" line (and the app
template) that said $clearRoutes is not inherited -- it now is.

Verified locally, view 605/0/0 + wheelstest 267/0/0 on lucee7, adobe2023 AND
adobe2025.

Refs #4194
Refs #4183

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…4194 PR2)

Replace each spec's hand-rolled route save/restore with $snapshotRoutes() /
$restoreRoutes() and drop the local $clearRoutes shims (now inherited from
wheels.WheelsTest, added in #4202). Several of these restored only a subset --
mapperSpec, mapperModernSpec, mapper/MapperSpec, mapper/MapperHardenerShouldSpec,
mapper/MapperNullPatternSpec and mapper/MapperRobustnessSpec each dropped
namedRoutePositions and/or staticRoutes -- so the shared helper also closes
those latent leaks.

PR2 of the staged migration; stacked on #4202 (the helper is not on develop
yet). Depends on #4202; merge order #4202 then this.

Verified locally: full core suite clean on lucee7 (6518 pass; the only
failures are the known TestClient port-mapped loopback artifacts, 0 real),
adobe2023 (6532/0/0) and adobe2025 (6532/0/0); 13 mapper bundles present.

Refs #4194

Signed-off-by: Peter Amiri <peter@alurium.com>
…ute helpers (#4194 PR3)

Migrate the remaining partial-clear / leak-class route specs onto
$snapshotRoutes() / $restoreRoutes() and drop their local $clearRoutes shims:

- dispatch/findMatchingRouteMegaSpec (partial-clear shim)
- global/urlforSpec (partial-clear shim; keeps its URLRewriting + obfuscateUrls
  saves, which are not route-table state)
- security/PaginationXssSpec (never saved staticRoutes -- the formsSpec leak
  class; its $clearRoutes CALL stays, now inherited)

PR3 of the staged migration; stacked on #4203 (helper chain not on develop
yet). Depends on #4203 (and #4202); merge order #4202, #4203, then this.

Verified locally, full core suite clean on lucee7 (6518 pass; only the known
TestClient port-mapped loopback artifacts, 0 real), adobe2023 (6532/0/0) and
adobe2025 (6532/0/0).

Refs #4194

Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ df1101a: APPROVE (stacked on #4203; merge after it, once matrix 37129072791 is green)

The three remaining specs are migrated mechanically:

  • dispatch/findMatchingRouteMegaSpec drops its partial $clearRoutes() shim (which never cleared namedRoutePositions) and uses snapshot and restore.
  • global/urlforSpec does the same and keeps its separate URLRewriting and obfuscateUrls saves.
  • security/PaginationXssSpec used to save and restore only routes and namedRoutePositions. Its staticRoutes leak is now covered by the snapshot, and its $clearRoutes() call resolves to the inherited full helper.

With #4202, #4203 and #4204 together, git grep 'function $clearRoutes' under vendor/wheels/tests finds no spec-level overrides left. Only the base helper remains (plus #4192's spec, deferred until it lands).

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Wheels Test Results

220 807 tests   216 087 ✅  1h 20m 56s ⏱️
 17 056 suites    4 720 💤
     32 files          0 ❌

Results for commit 8a886a8.

♻️ This comment has been updated with latest results.

Base automatically changed from peter/snapshot-routes-mapper-4194 to develop October 5, 2026 07:07
bpamiri and others added 2 commits October 5, 2026 00:09
…tes-leaks-4194

# Conflicts:
#	cli/lucli/templates/app/CLAUDE.md
#	docs/consumer-ai/CLAUDE.md
#	vendor/wheels/WheelsTest.cfc
Migrating urlforSpec onto the shared $snapshotRoutes/$restoreRoutes helpers
brought urlForCache into the restored key set. urlForCache is a lazily-created
persistent app-scope memo (routing.cfm), so when it was absent at snapshot time
$restoreRoutes deleted it on the afterEach — cascading so every test ended with
it gone. The issue-2955 'memo lives in application scope so it survives across
requests' test then found the memo absent (Expected [false] to be true).

Ensure urlForCache exists before the snapshot in the outer beforeEach, so the
full-state restore preserves the key instead of deleting it — matching the
framework's treatment of it as a persistent memo. All assertions are unchanged;
the issue-2955 block still manages the cache contents via its own save/restore.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri
bpamiri marked this pull request as ready for review October 5, 2026 07:24
@bpamiri bpamiri added this to the v4.2.0 milestone Oct 5, 2026
@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ 8a886a8: code APPROVE (final sign-off after matrix run 37277585624 and PR CI)

This moves the last three specs with their own route-state code onto the shared helpers: 3 files, +14/−29, and each local $clearRoutes() is deleted.

  • findMatchingRouteMegaSpec. beforeAll/afterAll now snapshot and restore everything. The old per-bundle restore covered only routes, staticRoutes and namedRoutePositions. Its local $clearRoutes() didn't reset namedRoutePositions at all, and the shared one does.
  • PaginationXssSpec. This fixes the leak itself. The old code restored only routes and namedRoutePositions, so the staticRoutes its $clearRoutes() emptied was never put back.
  • urlforSpec. It now snapshots and restores everything, alongside the URLRewriting / obfuscateUrls restores it keeps.

Should the urlForCache fix live in the shared helper? No, I'd keep it in the spec.

  • What happened. $restoreRoutes() restores exactly: a managed key that was absent at snapshot time is deleted. That's the right behaviour, because every framework read of the app-scope urlForCache is guarded and creates it lazily (routing.cfm L529–532, Mapper.cfc L267, the reload path at L456). So deleting it puts the app back in a real state: no URL generated yet.
  • The spec is order-dependent. What broke is the issue-2955 case "memo lives in application scope". It asserts StructKeyExists(application.wheels, "urlForCache") before its first urlFor() call. That only passed because an earlier spec had already created the cache, so it's an order dependence in that one test, not something the helper gets wrong.
  • No other spec hits it. I grepped every spec for urlForCache. The only other app-scope users (linkToUrlForDifferentialSpec) guard with StructKeyExists before clearing, and initializeRequestScopeSpec checks the request-scope cache that $initializeRequestScope() itself creates.
  • Moving it into the helper (for example "never delete urlForCache, only restore its contents") would make every spec quietly depend on that cache existing, and would hide the next order-dependent assertion instead of exposing it.

Optional, not blocking: the guard sits in the outer beforeEach, so it runs for every urlforSpec case. Moving it into the issue-2955 describe's beforeEach, or making that test assert after the first urlFor(), would keep it next to the one test that needs it. Fine as is.

@bpamiri

bpamiri commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ 8a886a8: APPROVE (final; matrix run 37277585624 read leg by leg, PR CI checked)

  • Matrix run 37277585624 is on this head, and all 32 legs plus rustcfml succeeded. From the junit artifact for each leg:
    • 6896–6913 tests per leg, with 0 failures and 0 errors on every leg.
    • All three migrated specs ran and passed on every leg, none skipped, including all six Adobe 2023 and all six Adobe 2025 legs and BoxLang: findMatchingRouteMegaSpec 12, urlforSpec 14 (including the issue-2955 cache cases) and PaginationXssSpec 14.
  • PR CI on this head: 18 pass, 3 skipped, 0 failing or pending, including Lucee 7 + SQLite (LuCLI) (run 37277536243).

@bpamiri
bpamiri merged commit d6831b0 into develop Oct 5, 2026
60 checks passed
@bpamiri
bpamiri deleted the peter/snapshot-routes-leaks-4194 branch October 5, 2026 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add shared $snapshotRoutes()/$restoreRoutes() helpers to WheelsTest

1 participant