TESTBOX-448: Fix MockBox $args() struct-order fragility + Set/Range support - #204
Merged
Merged
Conversation
…e value normalization
normalizeArguments() built its arg hash from struct.toString()/Java
Map.toString() for non-simple values, whose output depends on HashMap
iteration order - never guaranteed in CFML. Two structurally-equal
structs built in different insertion order could hash differently,
so $args() would silently miss a match. Latent for years; Lucee 7.1's
new ConcurrentHashMap-backed struct implementation (LDEV-5098) made
the fragility consistently visible.
Fix: normalizeValue() recursively canonicalizes composite argument
values instead of relying on raw Map/struct toString():
- Structs: keys sorted via java.util.TreeMap (order-independent),
values recursively normalized, then the whole thing is JSON-encoded
rather than hand-joined with bare "," / "=" / "{}" - hand-joining
let a string value containing those characters collide with a
completely different struct that happened to serialize to the same
raw text (e.g. {a:"1,b=2"} and {a:1,b:2} previously hashed
identically). JSON escaping closes that off for both structs and
arrays.
- BoxLang Range: canonicalized via toString(), which fully captures
bounds/step/exclusivity ("1..5" vs "1>..<5" vs "1..10.step(3)" all
differ) - never iterated/materialized, since ranges can be huge or
unbounded (an open-start range even throws if you try to iterate
it). Checked before the array branch, since isArray() is true for
a Range.
- BoxLang Set: unordered by definition, so elements are normalized
then sorted before JSON-encoding - the same order-independence
struct keys get, applied to set elements. Type-tagged (as is Range)
so a Set can never collide with an Array/string holding equivalent
content.
- CFC values: unchanged, still serialized via getMetadata() (moved
into the same recursive helper so nested CFCs inside structs/arrays
get the same treatment as top-level CFC args).
Range/Set checks are gated behind a computed IS_BOXLANG flag so
isRange()/isBoxSet() - which don't exist on Lucee/Adobe - are never
even attempted on those engines; short-circuit evaluation keeps the
whole branch inert there.
Tests: cross-engine coverage (struct order-independence, CFC-in-struct,
deep struct>array>struct nesting, and the delimiter-collision
regression) added to tests/specs/mockbox/MockBoxTest.cfc. Set/Range
coverage added as tests/specs/mockbox/MockBoxSetRangeTest.bx - a .bx
file, since BoxLang's `..` range operator isn't valid CFML syntax at
all on Lucee/Adobe (a parse-time failure, not just a missing-BIF one)
and TestBox's own bundle discovery already skips *.bx files entirely
on non-BoxLang engines, so this file is never compiled or run there.
Verified against BoxLang v1.17.0+58 end-to-end via the real
MockBox/$args()/createStub() API (not just the isolated normalization
logic): struct order-independence, delimiter-collision non-match, Set
order-independence across all three backing variants, Set-vs-Array
non-collision, Set-of-structs, Range match/non-match (bounds, step,
exclusivity), Range-vs-string non-collision, and unbounded-range
normalization all pass. Self-ran the full MockBoxTest + new
MockBoxSetRangeTest bundles (41 specs) - all new tests green; the
only 2 errors are a pre-existing, unrelated MockGenerator interface-stub
issue confirmed present on unmodified development too.
Supersedes #194 (external contributor PR for the same ticket): fixes
the delimiter-collision bug in that PR's normalizeValue() rewrite
(verified reproducible there against the real code, confirmed absent
on the current merged development baseline) and adds BoxLang Set/Range
support, which #194 did not handle.
Inline the struct literal directly into save() instead of a separate var declaration, avoiding a miscomputed manual alignment column against the adjacent var statement.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
normalizeArguments()built its arg hash fromstruct.toString()/JavaMap.toString()for non-simple values, whose output depends on HashMap iteration order — never guaranteed in CFML. Two structurally-equal structs built in different insertion order could hash differently, so$args()would silently miss a match. Latent for years; Lucee 7.1's newConcurrentHashMap-backed struct implementation (LDEV-5098) made the fragility consistently visible.normalizeValue()recursively canonicalizes composite argument values instead of relying on raw Map/structtoString().SetandRangetypes, cross-engine-safe (gated so they're never even attempted on Lucee/Adobe, where the relevant BIFs don't exist).Details
Structs/Arrays — keys sorted via
java.util.TreeMap(order-independent), values recursively normalized, then the whole thing is JSON-encoded rather than hand-joined with bare,/=/{}/[]. Hand-joining lets a string value containing those characters collide with a completely different struct that happens to serialize to the same raw text — e.g.{a:"1,b=2"}and{a:1,b:2}previously normalized to the identical string and therefore hashed identically, letting$args()false-match. This affects any struct/array containing an ordinary string with a comma,=, or bracket in it (names, notes, CSV-ish data — not an exotic edge case). JSON escaping closes that off.BoxLang
Range— canonicalized via.toString(), which fully captures bounds/step/exclusivity ("1..5"vs"1>..<5"vs"1..10.step(3)"all differ). Never iterated/materialized — ranges can be huge or unbounded, and an open-start range even throws if you try to iterate it. Checked before the array branch, sinceisArray()istruefor aRange.BoxLang
Set— unordered by definition (evenlinked/sortedvariants can differ from another equal set built differently), so elements are normalized then sorted before JSON-encoding — the same order-independence struct keys get, applied to set elements. Type-tagged (as isRange) so aSetcan never collide with anArray/string holding equivalent content.CFC values — unchanged behavior, still serialized via
getMetadata(), now inside the same recursive helper so nested CFCs inside structs/arrays get the same treatment as top-level CFC args.isRange()/isBoxSet()are gated behind a computedvariables.IS_BOXLANGflag so they're never even attempted on Lucee/Adobe (short-circuit evaluation keeps the branch fully inert there).Test plan
tests/specs/mockbox/MockBoxTest.cfc(cross-engine): struct order-independence, CFC-in-struct, deep struct→array→struct nesting, and a new delimiter-collision regression test.tests/specs/mockbox/MockBoxSetRangeTest.bx(BoxLang-only, new file): Set order-independence across all three backing variants, Set-vs-Array non-collision, Set-of-structs, Range match/non-match (bounds/step/exclusivity), Range-vs-string non-collision, unbounded-range normalization. This is a.bxfile rather than.cfcbecause BoxLang's..range operator isn't valid CFML syntax at all on Lucee/Adobe (a parse-time failure, not just a missing-BIF one) — TestBox's own bundle discovery already skips*.bxfiles entirely on non-BoxLang engines (getSpecPaths()inTestBox.cfc), so this file is simply never compiled or run there.MockBox/$args()/createStub()API (not just the isolated normalization logic) — all new tests pass. Self-ran the fullMockBoxTest+ newMockBoxSetRangeTestbundles (41 specs); the only 2 errors are a pre-existing, unrelatedMockGeneratorinterface-stub issue, confirmed present on unmodifieddevelopmenttoo (not introduced by this change).Relation to #194
Supersedes #194 (external contributor PR for the same ticket): that PR's rewrite fixed struct-order independence but introduced the delimiter-collision bug described above (verified reproducible against its actual code, and verified absent on the current
developmentbaseline, so it's a regression in that PR's approach, not a pre-existing issue). This PR also adds Set/Range support, which #194 didn't handle.Generated by Claude Code