Skip to content

Commit 950a7f8

Browse files
wheels-bot[bot]github-actions[bot]bpamiri
authored
fix(mapper): reject redundant namespace prefix in to= and controller= (#2794)
* fix(mapper): reject redundant namespace prefix in to= and controller= Inside `.namespace("foo")` (or equivalent `.scope()` / `.package()`), writing `to="foo/dashboard##index"` instead of `to="dashboard##index"` silently produced a `foo.foo/dashboard` controller path that downstream got flattened to a `Foodashboard`-style class lookup with an opaque `Wheels.ViewNotFound` error — leaving users to chase the symptom rather than the route definition. `$match()` now detects when the parsed controller starts with the scope's package converted to slash form and throws `Wheels.MapperArgumentInvalid` at registration time. The error names the namespace and the offending value and points at the correct shorter form. Fixes #2791 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(mapper): address Reviewer A/B consensus findings (round 1) - Snapshot `local.fromTo` / `local.originalTo` before the `to=` parse block so the error detail can distinguish `to=` vs direct `controller=` callers (Reviewer A nit). - Add `Len(arguments.package) > 0` to the guard's outer condition so an empty package does not yield `prefix = "/"` and spuriously reject controllers whose path starts with a slash (Reviewer A response, Reviewer B round-1 missed-issue). - Collapse multi-line block comments above the guard in `matching.cfc` and above the new `it()` group in `MatchingSpec.cfc` to one-liners to comply with CLAUDE.md style (both reviewers). - Add a spec asserting `$match()` with `package = ""` and a controller starting with `/` is not falsely rejected. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(mapper): address Reviewer A/B consensus findings (round 2) - vendor/wheels/mapper/matching.cfc:328 — change local.hh = "##" to local.hh = "####" so the error-suggestion detail renders as to="dashboard##index" (source-correct CFML), not to="dashboard#index" (Reviewer A finding, Reviewer B verified). - vendor/wheels/tests/specs/mapper/MatchingSpec.cfc:241–242 — collapse the 2-line comment inside the "Allows controllers..." spec body to a single line per CLAUDE.md "one short line max" rule (both reviewers). Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> --------- Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Signed-off-by: Peter Amiri <peter@alurium.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Peter Amiri <peter@alurium.com>
1 parent 343ae6f commit 950a7f8

3 files changed

Lines changed: 102 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ All historical references to "CFWheels" in this changelog have been preserved fo
2222

2323
### Fixed
2424

25+
- Routes registered inside `.namespace("foo")` (or equivalent `.scope()` / `.package()`) with a redundant namespace prefix in the controller path — e.g. `to="foo/dashboard##index"` instead of `to="dashboard##index"` — previously silently produced a `foo.foo/dashboard` lookup that downstream flattened to a `Foodashboard`-style class name with an opaque `Wheels.ViewNotFound` error. The Mapper now rejects this at route-registration time with `Wheels.MapperArgumentInvalid`, naming the namespace and the offending value and pointing at the correct shorter form, so users can find the bad route definition instead of chasing the symptom (#2791)
2526
- `WheelsTest` auto-bind missed user-defined global helpers added via `include` in `app/global/functions.cfm`. The pseudo-constructor used `getMetaData(application.wo).functions`, which only enumerates methods declared directly on the CFC and skips symbols merged in via `cfinclude`. Specs that called custom helpers (e.g. `can()`, `hasRole()`) had to manually rebind each one in `beforeAll()`. The auto-bind now iterates `application.wo` as a struct and binds every UDF via `isCustomFunction()`, preserving the existing public-only filter for declared methods (#2790)
2627
- Model layer SELECT clause builder now routes column identifiers through the adapter's `$quoteIdentifier`, so reserved-word column names (e.g. `key`, `order`, `group`) survive on every supported dialect instead of breaking `findAll` / `findOne` / dynamic finders with cryptic SQL syntax errors. The WHERE / ORDER BY paths already quoted columns; `$createSQLFieldList` and the empty-pagination column-list extraction in `read.cfc` now match.
2728
- `wheels packages install <name>` is now a transparent alias for `wheels packages add <name>` on every caller path that actually reaches `Module.cfc` (stdio MCP server, scripted in-process clients, spec suite). Previously the dispatch layer's `case "install":` branch printed a yellow warning to stdout and returned `""` without installing anything — so even though `PackagesMainCli.install()` itself had been a true alias for `add()` since #2729, any caller routing through the CLI module's verb dispatch silently no-op'd. The shell-facing `wheels packages install <name>` form is still intercepted by LuCLI's built-in extension installer upstream of module dispatch and remains broken on that path (and is documented as such in the module-owned help text), but MCP tool calls and programmatic callers now behave identically to `add`. Both branches now share a single fall-through body so the validation, error shape, and install behavior cannot drift apart again.

vendor/wheels/mapper/matching.cfc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,12 +303,42 @@ component {
303303
}
304304

305305
// Interpret "to" as "controller##action".
306+
local.fromTo = false;
307+
local.originalTo = "";
306308
if (StructKeyExists(arguments, "to")) {
309+
local.fromTo = true;
310+
local.originalTo = arguments.to;
307311
arguments.controller = ListFirst(arguments.to, "##");
308312
arguments.action = ListLast(arguments.to, "##");
309313
StructDelete(arguments, "to");
310314
}
311315

316+
// Guard: reject redundant namespace prefix in to=/controller= (#2791).
317+
if (
318+
StructKeyExists(arguments, "package")
319+
&& Len(arguments.package) > 0
320+
&& StructKeyExists(arguments, "controller")
321+
&& Find("/", arguments.controller)
322+
) {
323+
local.packageAsPath = Replace(arguments.package, ".", "/", "all");
324+
local.prefix = local.packageAsPath & "/";
325+
if (Len(arguments.controller) > Len(local.prefix) && Left(arguments.controller, Len(local.prefix)) == local.prefix) {
326+
local.stripped = Mid(arguments.controller, Len(local.prefix) + 1, Len(arguments.controller) - Len(local.prefix));
327+
local.actionForMsg = StructKeyExists(arguments, "action") ? arguments.action : "action";
328+
local.hh = "####";
329+
if (local.fromTo) {
330+
local.detail = "Got controller=""" & arguments.controller & """ (from to=""" & local.originalTo & """). The namespace prefix is added automatically — use to=""" & local.stripped & local.hh & local.actionForMsg & """ instead.";
331+
} else {
332+
local.detail = "Got controller=""" & arguments.controller & """ (passed as controller=). The namespace prefix is added automatically — use controller=""" & local.stripped & """ (or to=""" & local.stripped & local.hh & local.actionForMsg & """) instead.";
333+
}
334+
Throw(
335+
type = "Wheels.MapperArgumentInvalid",
336+
message = "Route inside `.namespace('#arguments.package#')` (or equivalent `.scope()` / `.package()`) uses a redundant namespace prefix in its controller path.",
337+
detail = local.detail
338+
);
339+
}
340+
}
341+
312342
// Pull route name from arguments if it exists.
313343
local.name = "";
314344
if (StructKeyExists(arguments, "name")) {

vendor/wheels/tests/specs/mapper/MatchingSpec.cfc

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,77 @@ component extends="wheels.WheelsTest" {
197197
expect(r[4]).toHaveKey("redirect")
198198
expect(r[5]).toHaveKey("redirect")
199199
});
200+
201+
// Guard against redundant namespace prefix in to=/controller= (#2791).
202+
it("Rejects to= with redundant namespace prefix", function(){
203+
expect(function() {
204+
m.$draw()
205+
.namespace("datapai")
206+
.get(name = "datapaiDashboard", pattern = "dashboard", to = "datapai/dashboard##index")
207+
.end()
208+
.end();
209+
}).toThrow(type = "Wheels.MapperArgumentInvalid");
210+
});
211+
it("Rejects controller= with redundant namespace prefix", function(){
212+
expect(function() {
213+
m.$draw()
214+
.namespace("admin")
215+
.$match(name = "dashboard", method = "get", controller = "admin/dashboard", action = "index")
216+
.end()
217+
.end();
218+
}).toThrow(type = "Wheels.MapperArgumentInvalid");
219+
});
220+
it("Rejects redundant prefix in nested namespaces", function(){
221+
expect(function() {
222+
m.$draw()
223+
.namespace("admin")
224+
.namespace("users")
225+
.get(name = "edit", pattern = "edit", to = "admin/users/profile##edit")
226+
.end()
227+
.end()
228+
.end();
229+
}).toThrow(type = "Wheels.MapperArgumentInvalid");
230+
});
231+
it("Rejects redundant prefix inside .package() too", function(){
232+
expect(function() {
233+
m.$draw()
234+
.package("admin")
235+
.get(name = "users", pattern = "users", to = "admin/users##index")
236+
.end()
237+
.end();
238+
}).toThrow(type = "Wheels.MapperArgumentInvalid");
239+
});
240+
it("Allows controllers whose name only happens to share a prefix with the namespace", function(){
241+
// "foobar/dashboard" inside .namespace("foo") is not a redundant prefix — "foobar" != "foo".
242+
m.$draw()
243+
.namespace("foo")
244+
.get(name = "dashboard", pattern = "dashboard", to = "foobar/dashboard##index")
245+
.end()
246+
.end();
247+
r = m.getRoutes();
248+
expect(r).toBeArray();
249+
expect(ArrayLen(r) >= 1).toBeTrue();
250+
});
251+
it("Accepts the correct (non-redundant) form inside a namespace", function(){
252+
m.$draw()
253+
.namespace("datapai")
254+
.get(name = "datapaiDashboard", pattern = "dashboard", to = "dashboard##index")
255+
.end()
256+
.end();
257+
r = m.getRoutes();
258+
expect(r).toBeArray();
259+
expect(ArrayLen(r) >= 1).toBeTrue();
260+
expect(r[1].controller).toBe("datapai.dashboard");
261+
expect(r[1].action).toBe("index");
262+
});
263+
it("Does not falsely reject when package is empty", function(){
264+
m.$draw()
265+
.$match(name = "edge", method = "get", pattern = "edge", controller = "/users", action = "index", package = "")
266+
.end();
267+
r = m.getRoutes();
268+
expect(r).toBeArray();
269+
expect(ArrayLen(r) >= 1).toBeTrue();
270+
});
200271
});
201272
}
202273

0 commit comments

Comments
 (0)