Skip to content

fix(csv-parse): refresh trim state after BOM detection - #537

Open
nk-batman21 wants to merge 1 commit into
adaltas:masterfrom
nk-batman21:fix/parse-bom-trim-state
Open

nk-batman21 wants to merge 1 commit into
adaltas:masterfrom
nk-batman21:fix/parse-bom-trim-state

Conversation

@nk-batman21

Copy link
Copy Markdown

Fixes #536.

When a UTF-16LE BOM changes the encoding, trimming still uses buffers prepared for the initial encoding. For example, parsing Buffer.from("\ufeff a , b ", "utf16le") with { bom: true, trim: true } returns [["愀", "戀"]] instead of [["a", "b"]].

Refresh the encoding-dependent trim buffers and lookahead sizes after BOM detection, preserving the existing parser state and byte offsets. Add sync and byte-sized streaming regressions for spaces and nonbreaking spaces with trim and ltrim.

Validation on Node.js 24.18.0:

  • All eight new cases fail before the fix and pass afterward; 370 additional encoding, Unicode whitespace, header and quoted-field controls pass.
  • Linux workspace build and csv-parse TypeScript check pass. Parser tests: 626 passing, three pending, excluding the unchanged group_columns_by_name validation assertion.
  • Changed-file ESLint, Prettier and git diff --check pass.

The full test command still fails that existing assertion: it expects "an boolean" while the source now returns "a boolean" after #525. The same failure was verified with pristine production code.

@pauloflp

Copy link
Copy Markdown

Independent validation on Node.js 24.22.1, comparing base f12e6862a41f4324a06a26c87a3e28031d4faa7f with head d7cb2a5b1b7927a6314a4a40ce6f31628db996d4:

  • Unquoted fields: 3,549 cases passed at the PR head, versus 2,427 passed / 1,122 failed at the base. The matrix covered UTF-8 and BOM-detected UTF-16LE; trim, ltrim, and rtrim; 19 whitespace characters (including U+1680, U+2000–U+200A, U+202F, U+205F, U+3000, U+FEFF); Greek and supplementary-plane field values; CRLF; every possible two-chunk byte split for each fixture, plus byte-at-a-time writes and sync parsing.
  • A separate quoted-field fixture still fails for all 118 tested byte splits across the two encodings. A smaller sync reproduction is below.
import { parse } from './packages/csv-parse/lib/sync.js';

parse(Buffer.from('\ufeff"a"\u2003,"b"', 'utf16le'), {
  bom: true, trim: true,
});
// CSV_NON_TRIMABLE_CHAR_AFTER_CLOSING_QUOTE
// Expected: [['a', 'b']]

parse(Buffer.from('"a"\u2003,"b"', 'utf8'), { trim: true });
// Same error, without BOM detection.

This quoted-field behavior is not introduced by this PR: both the base and head also fail when parsing the same BOM-free fixture with explicit encoding: 'utf16le', and both fail the UTF-8 control. ASCII space after a closing quote also fails in UTF-16LE.

The remaining boundary appears to be in the field-append branch: after a closing quote, rappend is false but lappend is true because the field contains data. A multibyte trim character is recognized, but pos is only advanced by its full length when lappend === false; the next byte is then treated as non-trimmable. This is a source-based explanation consistent with the reproductions, rather than a proposed change to this PR.

The independent matrix supports the encoding-state refresh for unquoted fields and identifies this separate after-quote boundary for follow-up; it should not hold up the existing BOM fix.

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.

csv-parse corrupts UTF-16LE fields when BOM detection is combined with trim

2 participants