From 2f0fc425d8d7fc81b4063ccde4b175de06af7dd5 Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Fri, 11 Sep 2026 00:27:23 -0500 Subject: [PATCH] Yield the final record when the input does not terminate it A file whose last line has no trailing newline is common enough that failing on it is a real defect, and the tokenizer failed on it twice over, for two different reasons. The unquoted path retried a single time and then gave up, so it could not tell "the terminator has not been read yet" from "there is no terminator and never will be". It now loops until either a terminator appears or the reader is exhausted, and distinguishes the two reasons a read comes back empty: a full buffer is a genuine ShortBuffer, while an exhausted reader means whatever remains in the buffer is the record's last field. The quoted path failed for its own reason. `untilClosingQuote` withholds a closing quote that lands as the last byte in the buffer, since it cannot yet tell a "" escape from the end of the field. At end of input there is nothing left to wait for, so it now asks again instead of erroring, which is what lets an input end on a quoted field. A record ending in a column separator was also dropping its trailing empty field, yielding one field where `a,\n` yields two. Reaching the field state with an exhausted reader can only mean a column separator was just consumed -- a record that starts there is sent to eof instead -- so that case now emits the empty field it owes. Errors are preserved where they belong: a field longer than the buffer and an unclosed quoted field both still fail with ShortBuffer, each with a test of its own so that the new end-of-input paths cannot swallow a real error. Closes the TODO about the last line with and without a newline. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01NGYDXszLdpBxhkfqqHp17B --- README.md | 9 ++++-- src/main.zig | 62 ++++++++++++++++++++++++++++---------- test/csv_tokenizer.zig | 67 +++++++++++++++++++++++++++++++++++------- 3 files changed, 109 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index ad89b6a..7aafdd3 100644 --- a/README.md +++ b/README.md @@ -173,9 +173,12 @@ next call to `next()`. Copy it if you need to keep it. otherwise `next()` fails with `error.ShortBuffer`. - An empty line is not skipped: it yields a single zero-length `field` followed by `row_end`, the same shape as a one-column row. -- **The last record must be terminated.** Input whose final record simply - stops, with no terminator after it, fails with `error.ShortBuffer` rather - than yielding that record. +- The final record need not be terminated. Input that simply stops yields its + last record as usual, so a file with no trailing newline reads the same as + one with it. A record ending in a column separator keeps its trailing empty + field, so `a,` is two fields exactly as `a,\n` is. +- An unclosed quoted field is an error (`error.ShortBuffer`), as is a field + longer than the buffer. ## Development diff --git a/src/main.zig b/src/main.zig index 385d355..4fbb6c6 100644 --- a/src/main.zig +++ b/src/main.zig @@ -146,6 +146,15 @@ pub const CsvReader = struct { return null; } + /// Take everything left in the buffer, leaving it empty. Only meaningful + /// once the underlying reader is exhausted: it is how the final field of + /// an input that ends without a record terminator is recovered. + pub fn takeRest(self: *Self) []u8 { + const rest = self.current; + self.current = self.current[rest.len..]; + return rest; + } + /// Tries to read more data from an underlying reader if buffer is not already full. /// If anything was read returns true, otherwise false. pub fn read(self: *Self) !bool { @@ -290,9 +299,16 @@ pub const CsvTokenizer = struct { next_status = switch (status) { .initial => if (try self.reader.read()) Status.row_start else Status.eof, .row_start => if (!try self.reader.ensureData()) Status.eof else Status.field, - .field => blk: { + .field => { if (!try self.reader.ensureData()) { - break :blk .row_end; + // Reaching `.field` with nothing left to read can + // only mean that a column separator was consumed and + // then the input ended, so the record has one last, + // empty field: `a,` is two fields, exactly as `a,\n` + // is. `.row_start` sends an exhausted reader to + // `.eof` instead, so a record never starts here. + self.status = .row_end; + return CsvToken{ .field = "" }; } return try self.parseField(); @@ -363,17 +379,25 @@ pub const CsvTokenizer = struct { if (first != '"') { var field = try self.reader.until(self.terminals()); - if (field == null) { - // force read - maybe separator was not read yet - const hasData = try self.reader.read(); - if (!hasData) { + while (field == null) { + // No terminator among what has been read so far, which means + // either that more is coming or that the input has run out. + const has_data = try self.reader.read(); + if (!has_data) { + if (self.reader.all_read) { + // The input ends without terminating its last + // record, so whatever remains in the buffer is that + // record's final field. Go straight to `.row_end`, + // which closes the record off an exhausted reader. + self.status = .row_end; + return CsvToken{ .field = self.reader.takeRest() }; + } + + // The buffer filled up without a terminator appearing. return CsvError.ShortBuffer; } field = try self.reader.until(self.terminals()); - if (field == null) { - return CsvError.ShortBuffer; - } } const terminator = (try self.reader.peek()).?; @@ -397,16 +421,24 @@ pub const CsvTokenizer = struct { // consume opening quote _ = try self.reader.char(); var quoted_field = try self.reader.untilClosingQuote(self.config.quote); - if (quoted_field == null) { - // force read - maybe separator was not read yet - const hasData = try self.reader.read(); - if (!hasData) { + while (quoted_field == null) { + const has_data = try self.reader.read(); + + // `untilClosingQuote` withholds a closing quote that is the + // last byte in the buffer, because it cannot yet tell a `""` + // escape from the end of the field. Once the reader is + // exhausted there is nothing left to wait for, so ask again + // rather than give up -- that is what lets an input end on a + // quoted field with no terminator after it. + if (!has_data and !self.reader.all_read) { return CsvError.ShortBuffer; } - // this read will fill the buffer quoted_field = try self.reader.untilClosingQuote(self.config.quote); - if (quoted_field == null) { + if (quoted_field == null and !has_data) { + // Nothing more was read and the field still has no + // closing quote: it is unclosed, or longer than the + // buffer. return CsvError.ShortBuffer; } } diff --git a/test/csv_tokenizer.zig b/test/csv_tokenizer.zig index 445f1a5..34c4a02 100644 --- a/test/csv_tokenizer.zig +++ b/test/csv_tokenizer.zig @@ -162,8 +162,6 @@ test "Quoted field with double quotes can be read on retry" { try expect(next == null); } -// TODO test last line with new line and without - /// Drive a tokenizer across `data` and assert the entire token stream, so a /// terminator case can be stated as the fields and record breaks it ought to /// produce. A `null` in `expected` stands for a `row_end`. @@ -306,18 +304,65 @@ test "A record separator may be any byte" { ); } -test "The final record must be terminated" { - // Pre-existing behavior, independent of which terminator is configured: - // input whose last record simply stops fails rather than yielding that - // record. Pinned here so that a future fix is a deliberate change. - var reader = std.Io.Reader.fixed("1,abc\r\n2,def"); +test "Final record need not be terminated" { + try expectStream("1,abc\r\n2,def", .{}, 64, &.{ "1", "abc", null, "2", "def", null }); + try expectStream("1,abc\n2,def", .{}, 64, &.{ "1", "abc", null, "2", "def", null }); +} + +test "Input of a single unterminated field" { + try expectStream("abc", .{}, 64, &.{ "abc", null }); +} + +test "Unterminated record ending in a column separator keeps the empty field" { + // `1,` is two fields, exactly as `1,\n` is. + try expectStream("1,", .{}, 64, &.{ "1", "", null }); + try expectStream("1,\n", .{}, 64, &.{ "1", "", null }); + try expectStream("a,b\r\nc,", .{}, 64, &.{ "a", "b", null, "c", "", null }); +} + +test "Unterminated record ending in a quoted field" { + try expectStream("1,\"abc\"", .{}, 64, &.{ "1", "abc", null }); + try expectStream("\"a\",\"b\"", .{}, 64, &.{ "a", "b", null }); +} + +test "Unterminated final field spanning a buffer refill" { + const data = "aaa,bbb\r\nccc,dddddddddddddddddddd"; + const expected: []const ?[]const u8 = &.{ "aaa", "bbb", null, "ccc", "dddddddddddddddddddd", null }; + + inline for (.{ 21, 22, 24, 28, 32, 64 }) |buffer_len| { + expectStream(data, .{}, buffer_len, expected) catch |err| { + std.log.warn("failed with buffer_len={d}\n", .{buffer_len}); + return err; + }; + } +} + +test "Unterminated final record under an explicit byte separator" { + try expectStream("1,abc\n2,def", .{ .row_sep = .{ .byte = '\n' } }, 64, &.{ "1", "abc", null, "2", "def", null }); + try expectStream("1,abc\r2,def", .{ .row_sep = .{ .byte = '\r' } }, 64, &.{ "1", "abc", null, "2", "def", null }); +} + +test "A field longer than the buffer is still a short buffer" { + // The unterminated-last-record path must not swallow the genuine error: + // here the input has not run out, the buffer has. + var reader = std.Io.Reader.fixed("aaaaaaaaaaaaaaaaaaaa,b\n"); + var buffer: [8]u8 = undefined; + var csv = try getTokenizer(&reader, &buffer, .{}); + + try testing.expectError(csv_mod.CsvError.ShortBuffer, csv.next()); +} + +test "Unterminated quoted field with a doubled quote" { + try expectStream("\"a\"\"b\"", .{}, 64, &.{ "a\"b", null }); + try expectStream("\"\"", .{}, 64, &.{ "", null }); + try expectStream("\"a\r\nb\"", .{}, 64, &.{ "a\r\nb", null }); +} + +test "An unclosed quoted field is an error" { + var reader = std.Io.Reader.fixed("1,\"abc"); var buffer: [64]u8 = undefined; var csv = try getTokenizer(&reader, &buffer, .{}); try expectToken(csv_mod.CsvToken{ .field = "1" }, try csv.next()); - try expectToken(csv_mod.CsvToken{ .field = "abc" }, try csv.next()); - try expectToken(csv_mod.CsvToken{ .row_end = {} }, try csv.next()); - try expectToken(csv_mod.CsvToken{ .field = "2" }, try csv.next()); - try testing.expectError(csv_mod.CsvError.ShortBuffer, csv.next()); } -- knotmirror 0.0.1