fix(csv-stringify): quote fields fusing with a multi-character separator#489
Open
spokodev wants to merge 1 commit into
Open
fix(csv-stringify): quote fields fusing with a multi-character separator#489spokodev wants to merge 1 commit into
spokodev wants to merge 1 commit into
Conversation
A field whose tail is a non-empty prefix of a multi-character delimiter or
record delimiter must be quoted, otherwise the separator appended after it
re-tokenizes the boundary on parse and the value is silently corrupted:
stringify([["a:", "b"]], { delimiter: "::" }) // a:::b
parse("a:::b", { delimiter: "::" }) // [["a", ":b"]]
The quoting predicate only checked `value.indexOf(separator) >= 0`, which
catches a field that contains the whole separator but misses this boundary
fusion. The same gap affected `record_delimiter`. Both now share an
`emits_separator` helper; single-character separators and non-fusing fields
are unaffected (no extra quoting). RFC 4180 §2.6, generalized to the library's
multi-character separators.
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.
When a field is serialized before a multi-character
delimiter(orrecord_delimiter), and the field’s tail is a non-empty prefix of that separator, the appended separator fuses with the field tail. On parse, the separator is then matched inside the original field, silently corrupting the data:The same happens for a multi-character record delimiter with the last field of a record:
The quoting decision used
value.indexOf(separator) >= 0, which quotes a field that contains the whole separator but misses this boundary fusion. Forcingquoted_string: truealready produces correct, round-trippable output, confirming the field simply needs quoting.The fix routes both
delimiterandrecord_delimiterthrough a smallemits_separator(value, separator)helper:The boundary check runs only for multi-character separators, so single-character delimiters and non-fusing fields are unaffected (no extra quoting) — e.g. delimiter
":x"with field"a:"stays unquoted, since":x"never starts inside"a:".Tests added in
option.delimiter.jsandoption.record_delimiter.js(fusing field is quoted; non-fusing field is not). Full suite passes (the one unrelated pre-existing failure inapi.callback.js, "catch error in end handler #386", is present onmasteras well).dist/is left for the release build, matching #484.