Skip to content

Pagination handles are unnamespaced dynamic keys in request.wheels — a handle matching a framework key aliases onto it #3339

Description

@bpamiri

Summary

pagination() and setPagination() store pagination state in request.wheels under a user-supplied handle name:

// vendor/wheels/Global.cfc:3915 (setPagination)
local.args = Duplicate(arguments);
StructDelete(local.args, "handle");
request.wheels[arguments.handle] = local.args;

// vendor/wheels/Global.cfc:3847 (pagination)
return request.wheels[arguments.handle];

handle defaults to "query", but it is an arbitrary caller-chosen string. Since CFML struct keys are case-insensitive, any handle that matches a framework-owned request.wheels key aliases onto it — the same class of bug as #3336, where the per-request finder cache keyed on the bare model name and a Tenant model collided with request.wheels.tenant.

Found while fixing #3336 (PR #3338); flagged there and by the bot reviewer as worth tracking separately.

Why this is lower severity than #3336

#3336 collided automatically — you got it just by naming a model Tenant, which is the name the framework's own docs use for the control-plane model. This one needs an app author to actively pass handle="tenant", so nobody hits it by accident in the way #3336 did.

That's the reason it wasn't bundled into #3338, not a reason it's harmless.

Impact

The framework-owned keys currently reachable in request.wheels:

Key Owner Effect of a colliding handle
tenant tenancy subsystem (TenantResolver, tenant(), $tenantDataSource(), Job, TenantMigrator) setPagination(handle="tenant") overwrites the resolved tenant with a pagination struct. Post-#3336 tenant() will now reject it for having no dataSource, so it degrades to "no tenant" rather than misrouting — but the tenant context is still destroyed for the rest of the request.
$queryCache per-request finder cache (as of #3336) handle="$queryCache" overwrites the whole cache namespace with a pagination struct; the next finder's StructKeyExists walk hits a struct with no per-model slots.
<query hash> model/callbacks.cfc:331 reverse lookup Collision requires guessing a hash; not realistic.

The reverse direction matters too: pagination() throws Wheels.QueryHandleNotFound only when showErrorInformation is on. In production a handle that happens to name a framework key returns that key's struct as though it were pagination state.

Suggested fix

Same shape as #3336 — namespace it:

  • setPagination()request.wheels.$pagination[handle]
  • pagination() → read from request.wheels.$pagination[handle], keeping the existing Wheels.QueryHandleNotFound behaviour

That also fixes the reverse direction for free, since a handle can then only ever resolve inside its own namespace.

Two call sites, both in Global.cfc. Note vendor/wheels/tests/specs/model/miscellaneousSpec.cfc:207 reads request.wheels[args.handle] directly and would need the same repoint, as requestQueryCacheSpec.cfc did in #3338.

Broader point

Both this and #3336 are instances of the same design issue: request.wheels is simultaneously a namespace for framework state (tenant, reloadRefusedReason, …) and a dumping ground for caller-keyed data. Every dynamic-key write into it is a potential collision. Reserving $-prefixed sub-structs per subsystem — as #3338 does for the query cache — is the pattern that scales; this issue is the remaining known instance.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions