Repository navigation
fix(dispatch): use portable abort keyword in public-component 404 branch - #3032
wheels-bot[bot] wants to merge 3 commits into
Conversation
A bare `cfabort;` statement is Lucee-only — Adobe CF (2018/2021/2023/2025) parses it as a reference to an undefined variable named cfabort and throws `Variable CFABORT is undefined` at runtime. The anti-fingerprinting 404 branch in Dispatch.cfc (issue #2233) ended with one, so a stock Adobe install served `GET /` and `GET /wheels/*` with an HTTP 500 error template in testing/production instead of the intended clean 404 Not Found. Switch the single bare-script occurrence in vendor/wheels to the portable `abort;` keyword, and add a structural guard spec that scans vendor/wheels framework code (comments stripped) for any further bare `cfabort;` statements. Refs #3029 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
… ref A bare `cfabort;` in script context crashes every Adobe CF version with `Variable CFABORT is undefined`. Adds cross-engine invariant #13 and a matching reference section in .ai/wheels/cross-engine-compatibility.md. Relates to the fix in #3032 (issue #3029). Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes a real Adobe-only crash in the dispatcher's anti-fingerprinting 404 branch (issue #3029) by replacing the Lucee-only bare cfabort; statement with the portable abort; keyword, and adds a structural guard spec to prevent regressions. The fix is correct, minimal, and well-evidenced; the guard spec is sound and follows the repo's established source-scan pattern. Verdict: comment — two minor, non-blocking robustness suggestions on the new spec.
Correctness
Verified clean — no findings. Supporting evidence for the verdict:
- The portable
abort;form has abundant prior art in framework code that already runs green on the Adobe matrix:vendor/wheels/Global.cfc:2493,vendor/wheels/Public.cfc:41,vendor/wheels/controller/csrf.cfc:63,vendor/wheels/controller/processing.cfc:24,vendor/wheels/events/EventMethods.cfc:282, among others. - I re-ran the guard's scan logic against HEAD: the only remaining
cfabort;text in non-testvendor/wheelscode (.cfcand.cfm) is inside the new//comment atvendor/wheels/Dispatch.cfc:400, which the spec's comment-stripping removes. The guard passes as claimed. - The test plan's claim that the new comment placement keeps the 404 contract inside
PublicComponentProductionSpec's scan window checks out: the!application.wheels.enablePublicComponentgate sits 252 characters beforestatuscode=404inDispatch.cfc, comfortably inside that spec's 400-char window (vendor/wheels/tests/specs/security/PublicComponentProductionSpec.cfc:199).
Tests
Two minor robustness suggestions on vendor/wheels/tests/specs/dispatch/BareCfabortStatementSpec.cfc — neither blocks merge:
-
Windows path-separator asymmetry in the tests-skip filter (
BareCfabortStatementSpec.cfc:50):if (FindNoCase("#root#/tests/", Replace(filePath, "\", "/", "all"))) {filePathis normalized to forward slashes, butroot(fromExpandPath("/wheels")) is not. On a Windows dev boxrootcontains backslashes, so the skip never matches, the spec scans itself, and it self-flags — the failure-message string literal at line ~66 ("Bare `cfabort;` is Lucee-only...") contains the exactcfabort;sequence the regex matches, and string literals survive comment-stripping. CI runs in Linux containers so this can't break the matrix, but a contributor running the suite on Windows would see a false failure. Suggested fix: normalize once —var normalizedRoot = Replace(root, "\", "/", "all");— and compare against that. -
The guard only scans
*.cfc, but the framework ships.cfmfiles with<cfscript>blocks (BareCfabortStatementSpec.cfc:39,filter = "*.cfc"): e.g.vendor/wheels/events/init/*.cfmall containcfscriptblocks where a barecfabort;would crash Adobe exactly the same way, yet would escape this guard. There are currently zero offenders in.cfm(verified at HEAD), so this is a coverage-widening suggestion, not a bug — a secondDirectoryListpass withfilter = "*.cfm"(cross-engine-safer than a pipe-delimited filter) would close the gap.
For completeness, three concerns I raised and then refuted during self-review (no action needed): reading a describe-scoped var closure from inside it() callbacks, the (?s) inline regex flag, and the //-strip regex's blindness to :// inside string literals — all three idioms appear verbatim in the CI-green prior-art spec vendor/wheels/tests/specs/cli/OnErrorFallbackGuardSpec.cfc (lines 25–47 and 145–152), so they're established patterns here.
Conventions / Docs / Commits / Security
Clean. Single commit fix(dispatch): use portable abort keyword in public-component 404 branch conforms to commitlint (valid type, 73-char header, sentence case) and carries a Signed-off-by: trailer matching the author identity. Changelog fragment changelog.d/3029-dispatch-bare-cfabort-adobe-crash.fixed.md uses the correct naming and fixed type with a complete bullet line — no direct CHANGELOG.md edit. Docs deferred to bot-update-docs.yml per the checklist, with a sensible CLAUDE.md cross-engine-invariant candidate noted in the PR body. The change strictly preserves the #2233 anti-fingerprinting contract (404 + text/plain + abort), so no security regression.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR replaces the single bare cfabort; in vendor/wheels/Dispatch.cfc's anti-fingerprinting 404 branch with the portable abort; keyword (fixing the Adobe-only 500 from #3029), adds a structural guard spec, and documents the gotcha in CLAUDE.md + .ai/wheels/cross-engine-compatibility.md. The fix is correct and complete — I verified the changed branch at vendor/wheels/Dispatch.cfc:403, confirmed no other bare cfabort; remains anywhere in vendor/wheels runtime code (the only .cfm occurrence is the portable <cfabort> tag form in public/views/cli.cfm:1085), and confirmed the existing PublicComponentProductionSpec source-inspection window still captures statuscode=404 (the gate-to-header distance is ~250 chars, inside its 400-char window, since the new comment was placed after the header call). Commits are conventional and DCO-signed matching the author. Verdict: comment — two minor test-robustness suggestions on the new guard spec, nothing blocking.
Tests
-
Asymmetric path normalization in the tests-tree skip can make the guard flag itself —
vendor/wheels/tests/specs/dispatch/BareCfabortStatementSpec.cfc:49:if (FindNoCase("#root#/tests/", Replace(filePath, "\", "/", "all"))) {The haystack (
filePath) is normalized to forward slashes, but the needle (rootfromExpandPath("/wheels"), line 35) is not. On any engine/OS whereExpandPathreturns backslashes (Windows dev machines running the core suite), the needle contains backslashes, the skip never matches, and the spec scans its own file — whose string literals at lines 34 and 65 contain the literalcfabort;, whichREFindNoCase("cfabort[[:space:]]*;", ...)matches after comment-stripping (string literals survive the strip). The spec would then report itself as the offender. CI runs Linux containers so this won't fire there, and the PR's local Lucee 7 run confirms the Linux path works — but the fix is one line: normalizerootwith the sameReplace(root, "\", "/", "all")before building the needle. -
The scan covers
*.cfconly —vendor/wheels/tests/specs/dispatch/BareCfabortStatementSpec.cfc:39(filter = "*.cfc"). A barecfabort;inside a<cfscript>block of a.cfmundervendor/wheels(e.g.public/views/,events/) would crash Adobe identically and evade the guard. There's no live gap today — I grepped the tree; the only.cfmhit is the tag form<cfabort>inpublic/views/cli.cfm:1085, which is portable — so this is future-proofing only:filter = "*.cfc|*.cfm"would close it. The.aidoc and spec header both say the guard "scans the framework source", which slightly oversells the*.cfc-only scope.
Everything else about the spec checks out against prior art: var fn = function(...) in a describe body is established in 14 existing spec files, the (?s) inline modifier is already used cross-engine in OnErrorFallbackGuardSpec.cfc:47, comment-stripping before source-matching follows CLAUDE.md Anti-Pattern #14, and the regex correctly excludes the <cfabort> and cfabort() forms (verified: > and ( both break the cfabort + whitespace + ; match).
Docs
- Stale PR checklist — the PR body leaves "AI Reference Docs" and "CLAUDE.md" unchecked as "handled separately by
bot-update-docs.yml", but commit3d220b21in this PR updates both (CLAUDE.mdcross-engine invariant #13 and the new section in.ai/wheels/cross-engine-compatibility.md). Worth ticking those boxes so the merge record is accurate. The docs themselves are accurate and well-placed, and the changelog fragment (changelog.d/3029-dispatch-bare-cfabort-adobe-crash.fixed.md) uses the fragment system correctly with no directCHANGELOG.mdedit.
Commits
Clean. fix(dispatch): use portable abort keyword in public-component 404 branch (71-char header) and docs: add cfabort; Lucee-only invariant to CLAUDE.md and cross-engine ref (74-char header) — both use allowlisted types, explain the "why", and carry Signed-off-by: trailers matching the commit author. The PR title is itself a valid conventional-commit header for the squash merge.
- BareCfabortStatementSpec: normalize root before the tests-tree skip so the guard does not scan (and self-flag) its own source on Windows where ExpandPath returns backslashes (review finding 1) - BareCfabortStatementSpec: scan *.cfm as well as *.cfc via a second DirectoryList pass (cross-engine-safer than a pipe-delimited filter) so a bare cfabort; inside a .cfm script block cannot evade the guard (finding 2) - reword in-spec comments to plain ASCII without a tag-open token, which the Lucee 7 parser rejected inside a // line comment - PR body: tick the AI Reference Docs and CLAUDE.md checklist boxes now satisfied by commit 3d220b2 (finding 3) Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied the Reviewer's findings from its review of
While verifying, the reworded in-spec comment originally contained a literal Test run (Lucee 7, full core suite via the local server): all bundles The "three concerns refuted during self-review" the reviewer listed were explicitly no-action, so nothing was changed there. The new commit will trigger a fresh Reviewer run on the updated SHA. This stage is opt-in: re-apply the |
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This round reviews head 7cfe842aa ("fix(test): address reviewer findings (round 1)"), which responds to the previous review of 3d220b21. Both code findings from that round are genuinely fixed: the tests-tree skip now compares a forward-slash-normalized needle against the normalized haystack (BareCfabortStatementSpec.cfc:40 builds normalizedRoot, used at line 69), and the scan now covers *.cfm as well as *.cfc via a second DirectoryList pass (lines 52–61). The stale PR checklist (finding 3) was also corrected — "AI Reference Docs" and "CLAUDE.md" are now ticked. I re-verified the underlying fix is still sound: abort; at vendor/wheels/Dispatch.cfc:403, the PublicComponentProductionSpec gate-to-statuscode=404 distance remains inside its 400-char window (the new comment sits after the header call), and a fresh grep of vendor/wheels runtime code finds no remaining bare cfabort; — the only non-comment occurrences are the portable tag form <cfabort> in public/views/cli.cfm:1085 and wheelstest/system/util/XMLConverter.cfc:255, neither of which matches the guard's cfabort[[:space:]]*; pattern (correctly, including the unskipped wheelstest/ tree). The spec's comment-stripping mirrors the canonical $stripCfmlComments() in cli/lucli/services/Analysis.cfc:388-394 line-for-line in approach, so it follows the documented Anti-Pattern #14 idiom rather than inventing a new one. Verdict: comment — one minor commit-message accuracy nit below; nothing blocking, and since the repo squash-merges (PR title is the linted landing header, and it's valid at 72 chars), it self-resolves at merge.
Commits
-
Commit body bullet describes a change that is not in the diff —
7cfe842aa, third bullet:reword in-spec comments to plain ASCII without a tag-open token, which the Lucee 7 parser rejected inside a // line comment
git diff fccf58e60..7cfe842aa -- vendor/wheels/tests/specs/dispatch/BareCfabortStatementSpec.cfccontains no comment rewording — only the root normalization, the*.cfmsecond pass, and the updated assertion message. And the surviving file still contains a tag-open token inside a//line comment (BareCfabortStatementSpec.cfc:22:// Strip tag comments (<!--- --->), block comments (/* */) ...) plus non-ASCII em-dashes at line 23, while the PR reports the dispatch layer green on Lucee 7 (110 pass / 0 fail). So the bullet both records work that isn't observable in the commit and asserts a Lucee 7 parser behavior the surviving source contradicts. Presumably it describes an intermediate unpushed draft that was fixed before committing — fine — but as written it plants a false engine-behavior claim ingit logfor anyone later researching Lucee comment-parsing quirks. No action required on this PR (the squash-merge subject is the PR title and it lints clean); just flagging so the inaccurate bullet isn't copied into docs or a future invariant entry.
Everything else verified clean this round: all three commits use allowlisted types with headers ≤ 100 chars (fix(dispatch): … 71, docs: … 74, fix(test): … 47), each carries a Signed-off-by: trailer matching its author (claude[bot] <41898282+claude[bot]@users.noreply.github.com>), the changelog fragment (changelog.d/3029-dispatch-bare-cfabort-adobe-crash.fixed.md) uses the fragment system with a valid fixed type and no direct CHANGELOG.md edit, and the CLAUDE.md invariant #13 + .ai/wheels/cross-engine-compatibility.md section added in 3d220b21 accurately describe the fix and its detection guard.
|
Superseded by #3037 (merged beca3f8) — same one-token Dispatch.cfc fix, structural guard spec, and CLAUDE.md Invariant 13, adversarially reviewed with live red→green verification on Adobe 2023 + Lucee 7. One piece of this draft not carried over: the .ai/wheels/cross-engine-compatibility.md entry — that doc gets the invariant via the guide-audit campaign's doc-alignment pass. |
The anti-fingerprinting 404 branch in
vendor/wheels/Dispatch.cfc(issue #2233) ended with a barecfabort;statement. That form is Lucee-only: Adobe CF (2018/2021/2023/2025) parses it as a reference to an undefined variable namedcfabortand throwsVariable CFABORT is undefinedat runtime. BecauseenablePublicComponent=falseintesting/productionroutes the stock homepage and everyGET /wheels/*request through this branch, a stock Adobe install crashed with an HTTP 500 error template instead of the intended clean404 Not Foundthat Lucee already produced. This switches the single bare-script occurrence to the portableabort;keyword and adds a structural guard spec that scansvendor/wheelsframework code (comments stripped per CLAUDE.md anti-pattern #14) for any further barecfabort;statements.Refs #3029
Type of Change
Feature Completeness Checklist
Signed-off-by:(git commit -s)vendor/wheels/tests/specs/dispatch/BareCfabortStatementSpec.cfc(failing -> passing structural guard)bot-update-docs.yml.ai/wheels/cross-engine-compatibility.mdupdated in commit3d220b213d220b21(bare tag-in-script statements likecfabort;are Lucee-only)changelog.d/3029-dispatch-bare-cfabort-adobe-crash.fixed.mdTest Plan
/wheels/Dispatch.cfcas the offender (Bare cfabort; is Lucee-only ...).cfabort;->abort;, the dispatch layer is green (110 pass / 0 fail / 0 error) and the existingsecurity/PublicComponentProductionSpecsource-inspection guard still passes (the explanatory comment was placed after thecfheader(statuscode=404)so the 404 contract stays inside that spec's scan window).environment=testing): on both Lucee 7 and Adobe 2023,GET /wheels/infoandGET /should return a clean404 Not Foundwith no error template. An actualabortcannot execute inside a spec without killing the TestBox runner, so the structural scan + this documented manual repro is the practical gate.