fix(csv-stringify): double regexp-metacharacter escape/quote literally#494
Merged
wdavidw merged 2 commits intoJul 2, 2026
Merged
Conversation
The escape and quote characters are configurable to any single character, but when a quoted field contained the escape or quote char, the code doubled occurrences via `new RegExp(escape, "g")` / `new RegExp(quote, "g")`. That interpolates the user char into a pattern, so metacharacters misbehave: `|` and `.` match everywhere and inject the doubled char at every position, `*`/`+`/`?` throw "Nothing to repeat", and `^`/`$` anchor and silently no-op. Replace the RegExp doubling with a literal `replaceAll` using a function replacer, so neither the pattern nor a `$` in the replacement is interpreted. This restores the round-trip invariant parse(stringify(x)) === x for any configured escape/quote character.
(cherry picked from commit 662726b)
Member
|
Thank you for the fix |
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.
A field that must be quoted and contains the configured
escapeorquotecharacter is corrupted, or throws, whenever that character is a regexp metacharacter.Steps to reproduce
escape/quoteare documented as any single character. When a quoted field contains the escape or quote char, the doubling was done withnew RegExp(escape, "g")/new RegExp(quote, "g"), interpolating the user character straight into a pattern.^and$behave as anchors and silently skip the doubling, so those characters also fail to round-trip.Root cause
The escape and quote doubling built a
RegExpfrom the user-configured character:Any regexp metacharacter is interpreted as regex syntax rather than a literal.
Fix
Replace the RegExp doubling with a literal
replaceAllusing a function replacer:The function replacer is required, not a plain replacement string: a
$in the replacement string is special ($&,$$, etc.), soescape: "$"would still be mangled by a string replacement. Returning the replacement from a function bypasses both the pattern-side and replacement-side interpretation.replaceAllwith a string pattern matches the character literally, so the previous\\special case is no longer needed.Authority
The round-trip invariant
parse(stringify(x, opts), opts) === x(RFC 4180 section 2.7, generalized to configurable escape/quote) must hold for any single-character escape/quote. Before the fix it is broken for regexp metacharacters (garbage output, or a thrown SyntaxError); after the fix it holds.Tests and suite status
Added cases in
test/option.escape.jscovering|,.,*,$as escape and.,|as quote. They fail on the current code (garbage output and a thrown SyntaxError) and pass with the fix.Full
csv-stringifysuite: 203 passing / 1 pending / 3 failing before, 205 passing / 1 pending / 1 failing after. The remaining failure is unrelated and pre-existing on a clean tree (api.callback"catch error in end handler, see #386"), where Node v26 reportsRangeError: Invalid string lengthinstead of the expectedERR_STRING_TOO_LONGmessage.