[Breaking] `parse`: text between and after balanced bracket groups is silently discarded, collapsing distinct keys onto one path
Summary
splitKeyIntoSegments collects balanced bracket groups and then jumps to the next [, so any ordinary text between or after those groups is silently discarded. Distinct raw keys therefore resolve to the same path, and the discarded text is lost with no error and no way for a caller to detect it.
qs.parse('a[b]tail0=x'); // { a: { b: 'x' } }
qs.parse('a[b]tail1=x'); // { a: { b: 'x' } } <- same path, different key
qs.parse('a[b]mid[c]=x'); // { a: { b: { c: 'x' } } } <- 'mid' dropped
qs.parse('a[b]=x'); // { a: { b: 'x' } }The current line is open = key.indexOf('[', close + 1); in lib/parse.js, but this is not a regression from that rewrite. The old regex splitter (child = /(\[[^[\]]*])/g driven by a global exec loop) skipped inter-group text by construction, exactly the same way. I checked out and ran v1.0.0, v2.0.0 and v6.0.0: all three return { a: { b: 'x' } } for a[b]tail0=x, so this dates to the original 2014 implementation.
Why file it
The supply of aliases is unbounded, so arbitrarily many distinct keys can be made to write to a single path. Combined with the strictMerge conflict wrap that became the default in 6.15.0, that had a knock-on effect on output nesting, which is being addressed separately on the merge side. This issue is only about the key splitting itself, which is the older and more fundamental half: two different keys should not silently become one, and a caller cannot currently tell that anything was dropped.
Why it is not a quick fix
I prototyped preserving the discarded text and it is semver-major, with at least three distinct problems:
- It breaks
test/parse.js:725("parses an object in dot notation"), which is a real expectation and not a characterization test. WithallowDots,user.email.becomesuser[email]., so preserving the trailing text resurrects a literal'.'key. Any fix has to decide what a trailing dot means underallowDotsfirst. a[b]constructor=vstarts getting silently dropped by the prototype guard instead of parsing.- Keys whose trailing text pushes them over the budget start throwing for existing
strictDepth: trueusers, converting a silent parse into a new synchronous throw.
It also changes output for four assertions added in f4938f5 (test/parse.js:325, :346, :347, :359), though those are explicitly characterization tests, written so a future change here shows up in the test diff.
Relationship to existing issues
- #558 and #560 cover unbalanced bracket groups. Note that #560 as currently designed would deliberately leave this route open:
test/parse.js:359filesa[b]extraunder "valid and stray-close bracket keys are unaffected by unbalanced-bracket handling". - #513 is the related round-trip problem for keys containing
[and].
Suggested resolution
Semver-major. Options are to preserve the discarded text as key content, or to treat a key with trailing garbage as a flat literal key the way a]b=v is already treated, or to throw. The allowDots interaction needs deciding either way.
Source: ljharb/qs