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()); }