fix(events): make debug-bar reload link subpath-aware (#3344) - #3371
Conversation
The debug bar's reload link was built from raw cgi.script_name, so under a subfolder (subpath) deployment it emitted /myapp/public/index.cfm/... links that the user's rewrite rules do not route. Compose the base from the resolved webPath plus the front-controller filename instead — the same idiom urlFor() uses — extracted into the unit-tested $buildDebugReloadUrl() helper in Global.cfc (public $-prefixed per cross-engine invariant 7). The helper preserves the previous behavior exactly for root installs (rewriting on and off, pinned byte-for-byte in the spec), keeps the request.cgi.path_info vs cgi.path_info branch, the rewriteFile strip, and the reload-param scrub, and falls back to the raw script name when webPath is not resolved yet (early boot/error paths). The CFML error page's displayed URL had the same class of defect and now uses the same webPath composition (plus HTML-encoding of the base). Fixes #3344 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <petera@pai.com>
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR fixes the debug-bar reload link (and the CFML error page's displayed URL) to be subpath-aware by composing the base from the resolved webPath + front-controller filename instead of raw cgi.script_name, extracting the logic into a new pure $buildDebugReloadUrl() helper with 12 TDD specs. I traced the change against the old inline composition and the existing urlFor() idiom and found no blocking issues — verdict: comment. The one open item is the author's own self-declared pending Adobe 2023 verification (below); I could not identify an Adobe-specific hazard in the helper myself, so this is a note, not a finding.
Correctness
The byte-identical-for-root-installs claim holds. Tracing the root case (scriptName="/index.cfm", webPath="/", rewriteFile="index.cfm"): "/" & ListLast("/index.cfm","/") → "/index.cfm", i.e. exactly the old cgi.script_name starting value, and the subsequent path_info append / rewriteFile strip / reload scrub are preserved verbatim from the removed debug.cfm block. The path_info-append comparison still uses the raw scriptName (arguments.pathInfo != arguments.scriptName, Global.cfc:2748), matching the old ... IS NOT cgi.script_name test. The empty-webPath fallback (Global.cfc:2743) reproduces the pre-fix output exactly (/wheelsproject1/public/posts?reload=), as pinned by the spec.
The webPath & ListLast(scriptName, "/") composition is the same idiom urlFor() uses at vendor/wheels/Global.cfc:1856, and webPath is guaranteed to end in / (Global.cfc:2660: local.normalized == "/" ? "/" : local.normalized & "/"), so the concatenation is safe. No off-by-one / null-deref concerns.
Cross-engine
Clean. The helper is pure string logic — no closures, no struct-literal array captures, no obj["key"]() calls, no application-scope function members, no Left(str, 0), and the for (local.i = ...) loop (Global.cfc:2757) is a plain body, not inside a finally (invariant 12 N/A). The optional-arg resolution via StructKeyExists(arguments, "webPath") rather than a runtime default-arg expression correctly mirrors the $resolveSubpathInclude prior art (Global.cfc:2686-2690). No parameter is named after a reserved scope (anti-pattern 11), and the helper is public $-prefixed per invariant 7.
Note (non-blocking): the PR body states "adobe2023 verification will be run by the maintainer session before merge." Since debug.cfm runs on every development request, that gate is worth honoring — but I traced no Adobe-2023/2025-specific construct in the added code, so I have no concrete failure to predict here.
Tests
Good coverage. BuildDebugReloadUrlSpec.cfc has 12 cases: root installs (rewriting on/off, query preservation, ?reload=/&reload=<env> scrubbing, empty path_info), subfolder installs (asserting no /public/ and no index.cfm), and the two defensive fallbacks. Uses the established g = application.wo (matches convertToStringSpec.cfc:5) and notToInclude (matches renderingSpec.cfc:334) patterns, extends wheels.WheelsTest (BDD, not legacy RocketUnit). Minor observation only: the debug.cfm path_info-source selection and the cfmlerror.cfm display are view templates and remain hand-checked rather than unit-covered — acceptable, since the extracted pure logic is what carries the risk and it is thoroughly tested.
Security
The error-page change (cfmlerror.cfm:80) additionally wraps the base fragment in EncodeForHTML(local.errorUrlBase), where the old line interpolated #Replace(cgi.script_name, ...)# raw — a small XSS hardening on the error page. Positive.
Docs / Commits
Changelog fragment changelog.d/debug-bar-reload-subpath.fixed.md is present with a valid .fixed type (not a direct CHANGELOG.md edit). Commit d200af9 is a valid conventional-commit header (fix(events): ..., ≤100 chars), is DCO Signed-off-by, and explains the why.
Nice work — well-scoped, behavior-preserving, and TDD'd.
Wheels Test Results 32 files 9 884 suites 20m 49s ⏱️ For more details on these failures and errors, see this check. Results for commit d200af9. |
1 participant
What / Why
Fixes #3344.
Issue #3344 is a re-conversion of discussion #2887, and everything in its body has already shipped: first-class subfolder support via the
subpathsetting (#2985), therootPathanchor fix (#3244), CLIwheels testunder subpaths (#3026), and the web app-test endpoint under subfolders (#3251). The test-runner-isolation half ("the test runner fights the live app's application scope") is tracked separately in #3025 and is intentionally NOT part of this PR.The one live, untracked defect (reported in the 2026-07-22 discussion comment that almost certainly prompted the re-conversion): the debug-bar reload link was built from raw
cgi.script_name, so under a subfolder (subpath) deployment it emitted/myapp/public/index.cfm/posts?reload=— a URL the user's rewrite rules don't route. This PR fixes that.Approach
vendor/wheels/events/onrequestend/debug.cfminto a new pure, unit-testable helper$buildDebugReloadUrl(scriptName, pathInfo, queryString[, webPath, rewriteFile])invendor/wheels/Global.cfc(public$-prefixed per cross-engine invariant 7; no parameter named after a reserved scope per anti-pattern 11; optionalwebPath/rewriteFileoverrides mirror the$resolveSubpathIncludeprior art for testability).webPath & ListLast(scriptName, "/")— the same idiomurlFor()uses (Global.cfc~1856) — instead of rawcgi.script_name. Therequest.cgi.path_infovscgi.path_infobranch (engines report it differently) stays indebug.cfm, which passes the selected value. The rewriteFile strip and reload-param scrub are preserved verbatim.application.wheels.webPathis missing/empty the helper falls back to the raw script name (the pre-fix behavior), and a missingrewriteFileskips the strip instead of throwing.vendor/wheels/events/onerror/cfmlerror.cfm), which had the same class of defect (plus HTML-encoding the previously raw base fragment).vendor/wheels/tests/specs/global/BuildDebugReloadUrlSpec.cfc(12 cases). The root-install expectations were pinned byte-for-byte against the OLD inline composition before the change (rewriting on, rewriting off, query-string preservation,?reload=/&reload=<env>scrubbing, empty path_info on Adobe), so non-subfolder installs render identically to before. Subfolder cases assert no/public/and noindex.cfmin the emitted link.changelog.d/debug-bar-reload-subpath.fixed.md.Test evidence
Component [wheels.Global] has no function with name [$buildDebugReloadUrl]) before implementation./?page=2&reload=<env>on a root install — byte-identical to the previous output.Cross-engine note
debug.cfmruns on every development request, so this is cross-engine-sensitive. The helper is pure string logic (no closures, no struct-literal captures, no tag wrappers), but adobe2023 verification will be run by the maintainer session before merge as an extra gate.🤖 Generated with Claude Code