diff --git a/CLAUDE.md b/CLAUDE.md index 27883e2a0..647c6ea14 100755 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,6 +54,20 @@ The framework must run on Lucee 5/6/7, Adobe CF 2018/2021/2023/2025, and BoxLang 15. **A parameter named `request` makes the bare `request` token resolve inconsistently on Adobe 2025.** In a function declaring a parameter named `request`, Adobe CF 2025 can resolve bare `request` to the built-in scope in one expression position and to `arguments.request` in another *within the same function* — so a guard written one way cannot protect an access written the other way. `if (StructKeyExists(request, "wheels")) { StructDelete(request.wheels, "tenant"); }` passed the guard and then threw `Element WHEELS is undefined in REQUEST`. Use `IsDefined("request.wheels.tenant")`, which string-resolves the whole dotted path in one evaluation, or assign before use (`if (!StructKeyExists(request, "wheels")) { request.wheels = {}; }` then write) — never mix the two forms. This hits **every middleware component**, because `wheels.middleware.MiddlewareInterface` mandates the signature `handle(required struct request, required any next)`; anti-pattern 11's "never name a parameter after a reserved scope" is unavailable there. Lucee 6/7, BoxLang and Adobe 2023 all resolve consistently, so **local Lucee green and Adobe 2023 smokes do NOT cover this** — only the Adobe 2025 matrix legs catch it, and `compat-matrix.yml` does not run on PRs (weekly cron + `workflow_dispatch`, `continue-on-error: true`). Hit by `TenantResolver.handle()` in [#3338](https://github.com/wheels-dev/wheels/pull/3338). +16. **A zero-argument call through the `application` scope breaks Adobe 2025's parser in statement position.** Inside a closure, `application.wo.$someMethod()` with an **empty** argument list — used as a bare statement or as the whole right-hand side of an assignment — throws at COMPILE time: `coldfusion.compiler.CFMLParserBase$MissingNameException: Invalid construct: Either argument or name is missing` ("When using named parameters to a function, each parameter must have a name"). Adobe appears to parse it as a script-style tag call and demand at least one attribute. This is the `application`-scope sibling of invariant 2. Verified boundaries — each of these compiles, so **do not "fix" them**: + - any argument at all: `application.wo.$get("showErrorInformation")` + - nested inside another call: `expect(application.wo.$statusCode()).toBe(418)` (long-standing in `renderingSpec`) + - chained further: `application.wo.mapper().resources("posts")` (`RoutePrecedenceSpec`) + - a non-`application` receiver, zero args, bare statement in a closure: `_controller.$clearCachableActions()` (`cachingSpec`), `strategy.logout()` (`SessionStrategySpec`), `local.c.$warnIfConfigSkipsSuper()` (`configSuperWarningSpec`) + + Two things make this expensive to diagnose. Adobe attributes the error to the **enclosing `describe(...)` line**, not the offending statement, so it reads like a broken test-block signature. And because the core suite compiles via `directory="wheels.tests.specs"`, one occurrence zeroes out **the entire engine leg** — adobe2025 reports `tests="0"` for every database while Lucee/BoxLang/Adobe 2023 stay green, and `compat-matrix.yml` does not run on PRs. In test code, ensure request state inline (`if (!StructKeyExists(request.wheels, "$pagination")) { request.wheels["$pagination"] = {} }`) rather than calling a void `$`-helper through `application.wo`; in framework code prefer helpers that **return** what they ensure, so callers write `local.store = $ensurePaginationStore();`. Hit by the #3339 pagination-namespace specs. + + Bisect this class of bug with a single probe against a running container instead of CI (~13s vs ~19min): + ```bash + curl -s "http://localhost:62025/wheels/core/tests?db=sqlite&format=json&cli=true" | \ + python3 -c "import json,sys; d=json.load(sys.stdin); print(d.get('totalPass','COMPILE FAIL'), d.get('RootCause',{}).get('snippet',''))" + ``` + Verify Adobe CF fixes locally before pushing — don't iterate via CI: ```bash curl -s "http://localhost:62023/wheels/core/tests?db=mysql&format=json" | \ diff --git a/changelog.d/3339-pagination-handle-namespace.fixed.md b/changelog.d/3339-pagination-handle-namespace.fixed.md new file mode 100644 index 000000000..de27486ce --- /dev/null +++ b/changelog.d/3339-pagination-handle-namespace.fixed.md @@ -0,0 +1 @@ +- Pagination handles are now stored under `request.wheels.$pagination[handle]` instead of directly in `request.wheels[handle]`. Handles are caller-supplied names, so the flat layout put arbitrary user input in the same case-insensitive keyspace as framework-owned request state, and the collision ran both ways. Writing: `setPagination(handle="tenant")` replaced the resolved tenant context with a pagination struct, and `handle="$queryCache"` did the same to the per-request finder cache — silently, since neither is validated. Reading: `pagination()` only checks that a handle exists when `showErrorInformation` is on, so in production an unknown handle that happened to name a framework key returned that key's struct as though it were pagination data. `request.wheels` currently holds around thirty-five framework-owned keys — including `params`, `execution`, `currentRoute`, `transactions`, `flashKeep` and `exception` — every one of which was reachable this way. Handles now resolve only inside their own sub-struct, so neither direction can cross over. `Wheels.QueryHandleNotFound` behaviour is unchanged (#3339) diff --git a/vendor/wheels/Global.cfc b/vendor/wheels/Global.cfc index f3524c294..a855c844d 100644 --- a/vendor/wheels/Global.cfc +++ b/vendor/wheels/Global.cfc @@ -3847,8 +3847,9 @@ return local.$wheels; * @handle The handle given to the query to return pagination information for. */ public struct function pagination(string handle = "query") { + local.store = $ensurePaginationStore(); if ($get("showErrorInformation")) { - if (!StructKeyExists(request.wheels, arguments.handle)) { + if (!StructKeyExists(local.store, arguments.handle)) { Throw( type = "Wheels.QueryHandleNotFound", message = "Wheels couldn't find a query with the handle of `#arguments.handle#`.", @@ -3856,7 +3857,33 @@ return local.$wheels; ); } } - return request.wheels[arguments.handle]; + return local.store[arguments.handle]; + } + + /** + * Internal function. + * Creates the reserved per-request pagination namespace if it doesn't exist yet. + * + * Pagination handles are caller-supplied names, so storing them directly in `request.wheels` + * put arbitrary user input in the same case-insensitive keyspace as framework-owned request + * state. A handle matching a framework key overwrote it, and — because `pagination()` only + * validates the handle when `showErrorInformation` is on — production reads of an unknown + * handle returned whatever framework struct happened to occupy that key. Both directions are + * closed by confining handles to their own sub-struct (#3339, same fix shape as #3336). + * + * Returns the namespace struct so callers can work through the returned reference instead of + * calling this as a bare statement — Adobe CF 2025's parser rejects a bare dotted call like + * `application.wo.$ensurePaginationStore()` in a script statement position (see the cross-engine + * note in CLAUDE.md). + */ + public struct function $ensurePaginationStore() { + if (!StructKeyExists(request, "wheels")) { + request.wheels = {}; + } + if (!StructKeyExists(request.wheels, "$pagination")) { + request.wheels["$pagination"] = {}; + } + return request.wheels["$pagination"]; } /** @@ -3924,7 +3951,8 @@ return local.$wheels; local.args = Duplicate(arguments); StructDelete(local.args, "handle"); - request.wheels[arguments.handle] = local.args; + local.store = $ensurePaginationStore(); + local.store[arguments.handle] = local.args; } /** diff --git a/vendor/wheels/tests/specs/controller/paginationHandleCollisionSpec.cfc b/vendor/wheels/tests/specs/controller/paginationHandleCollisionSpec.cfc new file mode 100644 index 000000000..84e024dd2 --- /dev/null +++ b/vendor/wheels/tests/specs/controller/paginationHandleCollisionSpec.cfc @@ -0,0 +1,139 @@ +/** + * Regression coverage for #3339. + * + * `setPagination()` / `pagination()` key themselves on a caller-supplied handle name (default + * `"query"`), which used to be written straight into `request.wheels`. CFML struct keys are + * case-insensitive, so a handle matching a framework-owned key collided with it in both + * directions: + * + * 1. Write: `setPagination(handle="tenant")` overwrote the resolved tenant context with a + * pagination struct. `handle="$queryCache"` did the same to the per-request finder cache. + * 2. Read: `pagination()` only validates the handle when `showErrorInformation` is on, so in + * production an unknown handle that happened to name a framework key returned that key's + * struct as though it were pagination state. + * + * Handles now live under the reserved `request.wheels.$pagination` sub-struct. Same fix shape as + * #3336, which moved the finder cache to `request.wheels.$queryCache`. + */ +component extends="wheels.WheelsTest" { + + function run() { + + g = application.wo; + + describe("pagination handle / framework key collision (##3339)", () => { + + // The whole core suite runs inside a single request, so request.wheels is shared across + // spec files. Only ever remove this spec's own handles — deleting the $pagination + // namespace wholesale would destroy handles other specs set up. + ownHandles = "articles,comments,tenant,$queryCache,noSuchHandleXYZ"; + + // Ensure the namespace inline rather than calling g.$ensurePaginationStore(): a + // zero-argument dotted call in statement position breaks Adobe CF 2025's parser. + beforeEach(() => { + originalShowErr = application.wheels.showErrorInformation; + originalCacheSetting = application.wheels.cacheQueriesDuringRequest; + StructDelete(request.wheels, "tenant"); + if (!StructKeyExists(request.wheels, "$pagination")) { + request.wheels["$pagination"] = {}; + } + paginationStore = request.wheels["$pagination"]; + for (var h in ListToArray(ownHandles)) { + StructDelete(paginationStore, h, false); + } + }) + + afterEach(() => { + application.wheels.showErrorInformation = originalShowErr; + application.wheels.cacheQueriesDuringRequest = originalCacheSetting; + StructDelete(request.wheels, "tenant"); + if (!StructKeyExists(request.wheels, "$pagination")) { + request.wheels["$pagination"] = {}; + } + paginationStore = request.wheels["$pagination"]; + for (var h in ListToArray(ownHandles)) { + StructDelete(paginationStore, h, false); + } + }) + + it("stores handles under the reserved namespace, not the bare key", () => { + g.setPagination(totalRecords = 100, currentPage = 2, perPage = 10, handle = "articles"); + + expect(StructKeyExists(request.wheels, "$pagination")).toBeTrue(); + expect(StructKeyExists(request.wheels["$pagination"], "articles")).toBeTrue(); + expect(StructKeyExists(request.wheels, "articles")).toBeFalse(); + }) + + it("round-trips pagination data through the namespace", () => { + g.setPagination(totalRecords = 100, currentPage = 2, perPage = 10, handle = "articles"); + var pg = g.pagination("articles"); + + expect(pg.totalRecords).toBe(100); + expect(pg.currentPage).toBe(2); + expect(pg.perPage).toBe(10); + expect(pg.totalPages).toBe(10); + }) + + // Write direction — a handle named after a framework key must not clobber it. + it("does not overwrite resolved tenant context when a handle is named tenant", () => { + request.wheels.tenant = {id = "acme", dataSource = "tenant_acme", config = {}, "$locked" = true}; + + g.setPagination(totalRecords = 50, currentPage = 1, perPage = 25, handle = "tenant"); + + expect(IsDefined("request.wheels.tenant")).toBeTrue(); + expect(request.wheels.tenant.id).toBe("acme"); + expect(request.wheels.tenant.dataSource).toBe("tenant_acme"); + expect(g.$tenantDataSource()).toBe("tenant_acme"); + }) + + it("does not overwrite the finder cache namespace when a handle is named $queryCache", () => { + application.wheels.cacheQueriesDuringRequest = true; + model("author").findAll(where = "lastName = 'Djurner'"); + var cachedBefore = StructCount(request.wheels["$queryCache"]["author"]); + + g.setPagination(totalRecords = 50, currentPage = 1, perPage = 25, handle = "$queryCache"); + + expect(StructKeyExists(request.wheels["$queryCache"], "author")).toBeTrue(); + expect(StructCount(request.wheels["$queryCache"]["author"])).toBe(cachedBefore); + }) + + // Read direction — the case showErrorInformation hides in production. + it("does not return a framework struct for an unknown handle when errors are hidden", () => { + application.wheels.showErrorInformation = false; + request.wheels.tenant = {id = "acme", dataSource = "tenant_acme", config = {}, "$locked" = true}; + + // Pre-fix this returned the tenant struct as though it were pagination data. + // It must now fail to resolve rather than hand back foreign state. + var result = {returnedTenant = false, threw = false}; + try { + var pg = g.pagination("tenant"); + result.returnedTenant = IsStruct(pg) && StructKeyExists(pg, "dataSource"); + } catch (any e) { + result.threw = true; + } + + expect(result.returnedTenant).toBeFalse(); + expect(result.threw).toBeTrue(); + }) + + it("still throws Wheels.QueryHandleNotFound for an unknown handle in development", () => { + application.wheels.showErrorInformation = true; + + expect(function() { + g.pagination("noSuchHandleXYZ"); + }).toThrow("Wheels.QueryHandleNotFound"); + }) + + it("keeps distinct handles isolated from each other", () => { + g.setPagination(totalRecords = 100, currentPage = 1, perPage = 10, handle = "articles"); + g.setPagination(totalRecords = 30, currentPage = 3, perPage = 5, handle = "comments"); + + expect(g.pagination("articles").totalRecords).toBe(100); + expect(g.pagination("comments").totalRecords).toBe(30); + expect(g.pagination("comments").currentPage).toBe(3); + }) + + }) + + } +} diff --git a/vendor/wheels/tests/specs/controller/requestSpec.cfc b/vendor/wheels/tests/specs/controller/requestSpec.cfc index 87e081ebb..9f1e74829 100644 --- a/vendor/wheels/tests/specs/controller/requestSpec.cfc +++ b/vendor/wheels/tests/specs/controller/requestSpec.cfc @@ -154,13 +154,22 @@ component extends="wheels.WheelsTest" { describe("Tests that pagination", () => { beforeEach(() => { - request.wheels["myhandle"] = {test = "true"} + // Ensure the namespace inline rather than calling application.wo.$ensurePaginationStore(): + // a zero-argument dotted call in statement position breaks Adobe CF 2025's parser. + if (!StructKeyExists(request.wheels, "$pagination")) { + request.wheels["$pagination"] = {} + } + paginationStore = request.wheels["$pagination"] + paginationStore["myhandle"] = {test = "true"} params = {controller = "dummy", action = "dummy"} _controller = application.wo.controller("dummy", params) }) afterEach(() => { - StructDelete(request.wheels, "myhandle", false) + // Delete only this spec's handle. The whole core suite runs in one request, so + // wiping the shared $pagination namespace would destroy other specs' handles. + paginationStore = request.wheels["$pagination"] + StructDelete(paginationStore, "myhandle", false) }) it("handle exists", () => { diff --git a/vendor/wheels/tests/specs/model/crudSpec.cfc b/vendor/wheels/tests/specs/model/crudSpec.cfc index da1e217ee..96caa4604 100644 --- a/vendor/wheels/tests/specs/model/crudSpec.cfc +++ b/vendor/wheels/tests/specs/model/crudSpec.cfc @@ -1477,10 +1477,10 @@ component extends="wheels.WheelsTest" { order = "id" ) - expect(request.wheels.pagination_test_1.CURRENTPAGE).toBe(1) - expect(request.wheels.pagination_test_1.TOTALPAGES).toBe(0) - expect(request.wheels.pagination_test_1.TOTALRECORDS).toBe(0) - expect(request.wheels.pagination_test_1.ENDROW).toBe(1) + expect(request.wheels["$pagination"].pagination_test_1.CURRENTPAGE).toBe(1) + expect(request.wheels["$pagination"].pagination_test_1.TOTALPAGES).toBe(0) + expect(request.wheels["$pagination"].pagination_test_1.TOTALRECORDS).toBe(0) + expect(request.wheels["$pagination"].pagination_test_1.ENDROW).toBe(1) expect(e.recordcount).toBe(0) }) @@ -1490,10 +1490,10 @@ component extends="wheels.WheelsTest" { /* 1st page */ e = user.findAll(select = "id", perpage = "2", page = "1", handle = "pagination_test_2", order = "id") - expect(request.wheels.pagination_test_2.CURRENTPAGE).toBe(1) - expect(request.wheels.pagination_test_2.TOTALPAGES).toBe(3) - expect(request.wheels.pagination_test_2.TOTALRECORDS).toBe(5) - expect(request.wheels.pagination_test_2.ENDROW).toBe(2) + expect(request.wheels["$pagination"].pagination_test_2.CURRENTPAGE).toBe(1) + expect(request.wheels["$pagination"].pagination_test_2.TOTALPAGES).toBe(3) + expect(request.wheels["$pagination"].pagination_test_2.TOTALRECORDS).toBe(5) + expect(request.wheels["$pagination"].pagination_test_2.ENDROW).toBe(2) expect(e.recordcount).toBe(2) expect(e.id[1]).toBe(r.id[1]) expect(e.id[2]).toBe(r.id[2]) @@ -1501,10 +1501,10 @@ component extends="wheels.WheelsTest" { /* 2nd page */ e = user.findAll(perpage = "2", page = "2", handle = "pagination_test_3", order = "id") - expect(request.wheels.pagination_test_3.CURRENTPAGE).toBe(2) - expect(request.wheels.pagination_test_3.TOTALPAGES).toBe(3) - expect(request.wheels.pagination_test_3.TOTALRECORDS).toBe(5) - expect(request.wheels.pagination_test_3.ENDROW).toBe(4) + expect(request.wheels["$pagination"].pagination_test_3.CURRENTPAGE).toBe(2) + expect(request.wheels["$pagination"].pagination_test_3.TOTALPAGES).toBe(3) + expect(request.wheels["$pagination"].pagination_test_3.TOTALRECORDS).toBe(5) + expect(request.wheels["$pagination"].pagination_test_3.ENDROW).toBe(4) expect(e.recordcount).toBe(2) expect(e.id[1]).toBe(r.id[3]) expect(e.id[2]).toBe(r.id[4]) @@ -1512,10 +1512,10 @@ component extends="wheels.WheelsTest" { /* 3rd page */ e = user.findAll(perpage = "2", page = "3", handle = "pagination_test_4", order = "id") - expect(request.wheels.pagination_test_4.CURRENTPAGE).toBe(3) - expect(request.wheels.pagination_test_4.TOTALPAGES).toBe(3) - expect(request.wheels.pagination_test_4.TOTALRECORDS).toBe(5) - expect(request.wheels.pagination_test_4.ENDROW).toBe(5) + expect(request.wheels["$pagination"].pagination_test_4.CURRENTPAGE).toBe(3) + expect(request.wheels["$pagination"].pagination_test_4.TOTALPAGES).toBe(3) + expect(request.wheels["$pagination"].pagination_test_4.TOTALRECORDS).toBe(5) + expect(request.wheels["$pagination"].pagination_test_4.ENDROW).toBe(5) expect(e.recordcount).toBe(1) expect(e.id[1]).toBe(r.id[5]) }) @@ -1568,10 +1568,10 @@ component extends="wheels.WheelsTest" { handle = "pagination_order_test_1" ) - expect(request.wheels.pagination_order_test_1.CURRENTPAGE).toBe(1) - expect(request.wheels.pagination_order_test_1.TOTALPAGES).toBe(13) - expect(request.wheels.pagination_order_test_1.TOTALRECORDS).toBe(250) - expect(request.wheels.pagination_order_test_1.ENDROW).toBe(20) + expect(request.wheels["$pagination"].pagination_order_test_1.CURRENTPAGE).toBe(1) + expect(request.wheels["$pagination"].pagination_order_test_1.TOTALPAGES).toBe(13) + expect(request.wheels["$pagination"].pagination_order_test_1.TOTALRECORDS).toBe(250) + expect(request.wheels["$pagination"].pagination_order_test_1.ENDROW).toBe(20) }) it("works with renamed primary key", () => { @@ -1590,10 +1590,10 @@ component extends="wheels.WheelsTest" { where = "description1 LIKE '%photo%'" ) - expect(request.wheels.pagination_order_test_1.CURRENTPAGE).toBe(1) - expect(request.wheels.pagination_order_test_1.TOTALPAGES).toBe(13) - expect(request.wheels.pagination_order_test_1.TOTALRECORDS).toBe(250) - expect(request.wheels.pagination_order_test_1.ENDROW).toBe(20) + expect(request.wheels["$pagination"].pagination_order_test_1.CURRENTPAGE).toBe(1) + expect(request.wheels["$pagination"].pagination_order_test_1.TOTALPAGES).toBe(13) + expect(request.wheels["$pagination"].pagination_order_test_1.TOTALRECORDS).toBe(250) + expect(request.wheels["$pagination"].pagination_order_test_1.ENDROW).toBe(20) }) it("works with parameterize set to false with numeric", () => { @@ -1607,10 +1607,10 @@ component extends="wheels.WheelsTest" { where = "id = 1" ) - expect(request.wheels.pagination_order_test_1.CURRENTPAGE).toBe(1) - expect(request.wheels.pagination_order_test_1.TOTALPAGES).toBe(1) - expect(request.wheels.pagination_order_test_1.TOTALRECORDS).toBe(1) - expect(request.wheels.pagination_order_test_1.ENDROW).toBe(1) + expect(request.wheels["$pagination"].pagination_order_test_1.CURRENTPAGE).toBe(1) + expect(request.wheels["$pagination"].pagination_order_test_1.TOTALPAGES).toBe(1) + expect(request.wheels["$pagination"].pagination_order_test_1.TOTALRECORDS).toBe(1) + expect(request.wheels["$pagination"].pagination_order_test_1.ENDROW).toBe(1) }) it("works with compound keys", () => { diff --git a/vendor/wheels/tests/specs/model/miscellaneousSpec.cfc b/vendor/wheels/tests/specs/model/miscellaneousSpec.cfc index f39bf3b7f..63b199d1f 100644 --- a/vendor/wheels/tests/specs/model/miscellaneousSpec.cfc +++ b/vendor/wheels/tests/specs/model/miscellaneousSpec.cfc @@ -202,9 +202,9 @@ component extends="wheels.WheelsTest" { function assert_pagination(required string handle) { args = arguments - expect(request.wheels).toHaveKey(args.handle) + expect(request.wheels["$pagination"]).toHaveKey(args.handle) - p = request.wheels[args.handle] + p = request.wheels["$pagination"][args.handle] StructDelete(args, "handle", false) for (i in args) { diff --git a/vendor/wheels/tests/specs/view/miscellaneousSpec.cfc b/vendor/wheels/tests/specs/view/miscellaneousSpec.cfc index 601f79ba2..f0ddd0cfd 100644 --- a/vendor/wheels/tests/specs/view/miscellaneousSpec.cfc +++ b/vendor/wheels/tests/specs/view/miscellaneousSpec.cfc @@ -255,7 +255,12 @@ component extends="wheels.WheelsTest" { }) it("is passing through class", () => { - request.wheels.testPaginationLinksQuery = {currentPage = 2, totalPages = 3} + // Inline rather than g.$ensurePaginationStore() — a zero-argument dotted call in + // statement position breaks Adobe CF 2025's parser. + if (!StructKeyExists(request.wheels, "$pagination")) { + request.wheels["$pagination"] = {} + } + request.wheels["$pagination"].testPaginationLinksQuery = {currentPage = 2, totalPages = 3} r = g.controller("dummy").paginationLinks(classForCurrent = "active", handle = "testPaginationLinksQuery") expect(r).toInclude("2")