Skip to content

fix(csv-stringify): quote fields containing a carriage return#485

Open
sarathfrancis90 wants to merge 1 commit into
adaltas:masterfrom
sarathfrancis90:fix/stringify-quote-carriage-return
Open

fix(csv-stringify): quote fields containing a carriage return#485
sarathfrancis90 wants to merge 1 commit into
adaltas:masterfrom
sarathfrancis90:fix/stringify-quote-carriage-return

Conversation

@sarathfrancis90

Copy link
Copy Markdown

A field holding a lone carriage return is written unquoted, because the quoting check only looks for the configured record_delimiter (\n by default). The parser treats both \r and \n as record delimiters, so the value gets split apart and the document no longer round-trips:

const { stringify } = require("csv-stringify/sync");
const { parse } = require("csv-parse/sync");

parse(stringify([["a\rb"]]));
// => [["a"], ["b"]]   (expected [["a\rb"]])

\n and \r\n are already quoted; only a bare \r slips through.

The fix: when the record delimiter is newline-based, quote any field that contains \r or \n. A custom, non-newline delimiter still leaves line breaks as data, so that behaviour is unchanged.

Found while fuzzing the stringifyparse round trip. Added a regression test; the escape-formulas test is updated since a \r-leading field is now correctly quoted. Full csv-stringify suite (202) and tsc pass.

A field holding a lone carriage return was written unquoted because the
quoting check only looked for the configured `record_delimiter` (`\n` by
default). The parser, however, treats both `\r` and `\n` as record
delimiters, so the value was split apart and the document no longer
round-tripped:

    parse(stringify([['a\rb']]))  // => [['a'], ['b']]  instead of [['a\rb']]

When the record delimiter is newline-based, quote any field that contains
`\r` or `\n`. A custom, non-newline delimiter still leaves line breaks
as data, so its behaviour is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant