Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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" | \
Expand Down
1 change: 1 addition & 0 deletions changelog.d/3339-pagination-handle-namespace.fixed.md
Original file line number Diff line number Diff line change
@@ -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)
34 changes: 31 additions & 3 deletions vendor/wheels/Global.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -3847,16 +3847,43 @@ 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#`.",
extendedInfo = "Make sure your `findAll` call has the `page` argument specified and matching `handle` argument if specified."
);
}
}
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"];
}

/**
Expand Down Expand Up @@ -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;
}

/**
Expand Down
139 changes: 139 additions & 0 deletions vendor/wheels/tests/specs/controller/paginationHandleCollisionSpec.cfc
Original file line number Diff line number Diff line change
@@ -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);
})

})

}
}
13 changes: 11 additions & 2 deletions vendor/wheels/tests/specs/controller/requestSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
56 changes: 28 additions & 28 deletions vendor/wheels/tests/specs/model/crudSpec.cfc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand All @@ -1490,32 +1490,32 @@ 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])

/* 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])

/* 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])
})
Expand Down Expand Up @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand Down
Loading
Loading