Skip to content

Exclude byte-order mark from ExtendedBufferedReader byte counting#622

Merged
garydgregory merged 1 commit into
apache:masterfrom
rootvector2:bom-byte-count
Jul 13, 2026
Merged

Exclude byte-order mark from ExtendedBufferedReader byte counting#622
garydgregory merged 1 commit into
apache:masterfrom
rootvector2:bom-byte-count

Conversation

@rootvector2

@rootvector2 rootvector2 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

getEncodedCharLength re-encodes each character with the configured CharsetEncoder to count its bytes, but the convenience encode call re-emits a byte-order mark on every invocation for charsets like UTF-16. Parsing a UTF-16 stream with setTrackBytes(true), for example an Excel Unicode export, counts every code unit as 4 bytes instead of 2, so CSVRecord.getBytePosition() grows at twice the true rate. Found by comparing reported byte offsets against the encoded prefix length across charsets.

Measure the per-call BOM once and subtract it from each encoded length. The count already lives in getEncodedCharLength, so keeping the adjustment there covers both the single-char and array read paths, and it stays a no-op for charsets that emit no BOM (UTF-8, UTF-16LE/BE, single-byte).

@garydgregory garydgregory changed the title exclude byte-order mark from ExtendedBufferedReader byte counting Exclude byte-order mark from ExtendedBufferedReader byte counting Jul 13, 2026
@garydgregory
garydgregory requested a review from Copilot July 13, 2026 15:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes byte-position tracking when parsing CSV data with trackBytes=true and a BOM-emitting charset encoder (notably UTF-16), preventing byte offsets from being inflated by a per-encode-call BOM.

Changes:

  • Compute and cache the per-call BOM length emitted by CharsetEncoder.encode(...) and subtract it from per-character encoded byte counts.
  • Add a regression test ensuring CSVRecord.getBytePosition() advances at the correct rate for UTF-16 when byte tracking is enabled.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/main/java/org/apache/commons/csv/ExtendedBufferedReader.java Measures per-call BOM emission once and subtracts it from byte-count calculations to keep bytesRead accurate for BOM-emitting encoders.
src/test/java/org/apache/commons/csv/CSVParserTest.java Adds coverage for correct byte-position advancement under UTF-16 with byte tracking enabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@garydgregory
garydgregory merged commit f1a35e7 into apache:master Jul 13, 2026
16 checks passed
@garydgregory

garydgregory commented Jul 13, 2026

Copy link
Copy Markdown
Member

Thank you @rootvector2 , merged 🚀
Would you please add a similar test for UTF-32 BE/LE?

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.

3 participants