fix: preserve CSV records and bound cell coordinates

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- CSV preserves carriage-return record endings and separator directives when
+  options are reapplied. Out-of-range coordinates and growth are rejected
+  before cell IDs or allocation sizes can wrap.
+
 - Quantity parsing and integer formatting are locale independent. Invalid or
   out-of-range integer magnitudes throw instead of silently converting.
 

src/odr/internal/csv/csv_document.cpp

@@ -14,8 +14,10 @@
 
 #include <algorithm>
 #include <istream>
+#include <limits>
 #include <memory>
 #include <ostream>
+#include <stdexcept>
 #include <string>
 #include <utility>
 
@@ -40,8 +42,18 @@ constexpr std::uint64_t column_mask = (std::uint64_t{1} << row_shift) - 1;
 constexpr std::uint64_t row_mask =
     (std::uint64_t{1} << (kind_shift - row_shift)) - 1;
 
+constexpr std::uint32_t max_columns = std::uint32_t{1} << row_shift;
+constexpr std::uint32_t max_rows = std::numeric_limits<std::uint32_t>::max();
+
+void check_position(const std::uint32_t column, const std::uint32_t row) {
+  if (column >= max_columns || row >= max_rows) {
+    throw std::out_of_range("csv position out of range");
+  }
+}
+
 ElementIdentifier make_id(const Kind kind, const std::uint32_t column = 0,
                           const std::uint32_t row = 0) {
+  check_position(column, row);
   return static_cast<std::uint64_t>(kind) << kind_shift |
          (static_cast<std::uint64_t>(row) & row_mask) << row_shift |
          (static_cast<std::uint64_t>(column) & column_mask);
@@ -69,11 +81,16 @@ class ElementAdapter final : public AdapterBase {
 
   [[nodiscard]] ElementType
   element_type(const ElementIdentifier element_id) const override {
+    const auto coordinates =
+        element_id & ((std::uint64_t{1} << kind_shift) - 1);
+    if ((coordinates >> row_shift) >= max_rows) {
+      return ElementType::none;
+    }
     switch (kind_of(element_id)) {
     case Kind::root:
-      return ElementType::root;
+      return coordinates == 0 ? ElementType::root : ElementType::none;
     case Kind::sheet:
-      return ElementType::sheet;
+      return coordinates == 0 ? ElementType::sheet : ElementType::none;
     case Kind::cell:
       return ElementType::sheet_cell;
     case Kind::text:
@@ -304,32 +321,23 @@ CsvDocument::CsvDocument(const abstract::File &file,
       encoding != TextEncoding::utf8 || bytes.starts_with("\xef\xbb\xbf");
   std::string text = encoding::to_utf8(bytes, encoding);
 
-  const std::size_t first_break = text.find('\n');
-  m_line_end = first_break != std::string::npos &&
-                       (first_break == 0 || text[first_break - 1] != '\r')
-                   ? "\n"
-                   : "\r\n";
   m_final_line_end = text.empty() || text.back() == '\n' || text.back() == '\r';
 
-  std::string_view remainder = text;
+  RecordReader reader(text, dialect);
+  std::vector<std::string> fields;
   if (skip_first_line) {
-    if (const std::size_t body = remainder.find_first_not_of(
-            "\r\n", remainder.find_first_of("\r\n"));
-        body != std::string_view::npos) {
-      remainder = remainder.substr(body);
-    } else {
-      remainder = {};
-    }
+    reader.read(fields);
   }
-
-  RecordReader reader(remainder, dialect);
-  std::vector<std::string> fields;
   std::uint32_t columns = 0;
   while (reader.read(fields)) {
+    if (fields.size() > max_columns || m_rows.size() >= max_rows) {
+      throw std::length_error("csv dimensions out of range");
+    }
     columns = std::max(columns, static_cast<std::uint32_t>(fields.size()));
     m_rows.push_back(fields);
   }
 
+  m_line_end = reader.line_end().empty() ? "\r\n" : reader.line_end();
   m_dimensions = {static_cast<std::uint32_t>(m_rows.size()), columns};
 
   // Walks the fields that exist rather than the rectangle they span: one wide
@@ -390,6 +398,7 @@ ValueType CsvDocument::value_type(const std::uint32_t column,
 
 void CsvDocument::set_cell(const std::uint32_t column, const std::uint32_t row,
                            std::string text) {
+  check_position(column, row);
   if (row >= m_rows.size()) {
     m_rows.resize(row + 1);
   }
@@ -424,6 +433,9 @@ void CsvDocument::insert_rows(const std::uint32_t row,
   if (row >= m_rows.size()) {
     return;
   }
+  if (count > max_rows - m_dimensions.rows) {
+    throw std::length_error("csv row count out of range");
+  }
   // a field per column, as the lines around it state
   m_rows.insert(m_rows.begin() + row, count,
                 std::vector<std::string>(m_dimensions.columns));
@@ -438,8 +450,7 @@ void CsvDocument::delete_rows(const std::uint32_t row,
   }
   m_rows.erase(m_rows.begin() + row,
                m_rows.begin() +
-                   std::min<std::size_t>(static_cast<std::size_t>(row) + count,
-                                         m_rows.size()));
+                   (row + std::min<std::size_t>(count, m_rows.size() - row)));
   m_dimensions.rows = static_cast<std::uint32_t>(m_rows.size());
   type_columns();
 }
@@ -449,6 +460,9 @@ void CsvDocument::insert_columns(const std::uint32_t column,
   if (column >= m_dimensions.columns) {
     return;
   }
+  if (count > max_columns - m_dimensions.columns) {
+    throw std::length_error("csv column count out of range");
+  }
   for (std::vector<std::string> &fields : m_rows) {
     if (column < fields.size()) {
       fields.insert(fields.begin() + column, count, std::string());

src/odr/internal/csv/csv_file.cpp

@@ -18,8 +18,9 @@ void check(const Dialect dialect) {
   if (dialect.separator == dialect.quote) {
     throw std::invalid_argument("csv separator equals its quote");
   }
-  if (dialect.separator == '\n' || dialect.separator == '\r') {
-    throw std::invalid_argument("csv separator is a line break");
+  if (dialect.separator == '\n' || dialect.separator == '\r' ||
+      dialect.quote == '\n' || dialect.quote == '\r') {
+    throw std::invalid_argument("csv delimiter is a line break");
   }
 }
 
@@ -33,6 +34,7 @@ CsvFile::CsvFile(std::shared_ptr<text::TextFile> file)
   }
   m_dialect = probe.dialect;
   m_separator_directive = probe.separator_directive;
+  check(m_dialect);
 }
 
 CsvFile::CsvFile(std::shared_ptr<abstract::File> file,
@@ -47,6 +49,9 @@ CsvFile::CsvFile(std::shared_ptr<abstract::File> file,
   if (options.separator.has_value()) {
     m_dialect.separator = *options.separator;
     check(m_dialect);
+    const Probe probe =
+        csv::probe(*m_file->file(), m_file->encoding(), m_dialect.quote);
+    m_separator_directive = probe.separator_directive;
     return;
   }
 

src/odr/internal/csv/csv_util.cpp

@@ -226,9 +226,15 @@ bool csv::RecordReader::read(std::vector<std::string> &fields) {
     } else if (c == m_dialect.separator) {
       end_field();
       started = true;
-    } else if (c == '\r') {
-      // CRLF, and a lone CR
-    } else if (c == '\n') {
+    } else if (c == '\r' || c == '\n') {
+      const std::size_t start = m_position;
+      if (c == '\r' && m_position + 1 < m_text.size() &&
+          m_text[m_position + 1] == '\n') {
+        ++m_position;
+      }
+      if (m_line_end.empty()) {
+        m_line_end = m_text.substr(start, m_position - start + 1);
+      }
       if (!started) {
         continue; // empty line
       }
@@ -255,6 +261,10 @@ bool csv::RecordReader::read(std::vector<std::string> &fields) {
 
 bool csv::RecordReader::unterminated() const noexcept { return m_unterminated; }
 
+std::string_view csv::RecordReader::line_end() const noexcept {
+  return m_line_end;
+}
+
 csv::Probe csv::probe(const std::string_view text, const bool complete,
                       const char quote) {
   Probe result;

src/odr/internal/csv/csv_util.hpp

@@ -35,11 +35,15 @@ class RecordReader final {
   /// Whether the text ran out inside a quoted field.
   [[nodiscard]] bool unterminated() const noexcept;
 
+  /// The first record delimiter, excluding line breaks inside fields.
+  [[nodiscard]] std::string_view line_end() const noexcept;
+
 private:
   std::string_view m_text;
   Dialect m_dialect;
   std::size_t m_position{0};
   bool m_unterminated{false};
+  std::string_view m_line_end;
 };
 
 /// What a probe made of a file's opening bytes.

test/src/internal/csv/csv_file_test.cpp

@@ -14,6 +14,7 @@
 #include <odr/internal/csv/csv_file.hpp>
 #include <odr/internal/csv/csv_util.hpp>
 
+#include <limits>
 #include <memory>
 #include <sstream>
 #include <string>
@@ -72,7 +73,8 @@ TEST(CsvFile, csv) {
 
 TEST(CsvProbe, a_consistent_field_count_is_what_makes_it_a_csv) {
   EXPECT_TRUE(probe("a,b,c\n1,2,3\n").is_csv);
-  EXPECT_TRUE(probe("a,b,c\n1,2,3").is_csv);        // no trailing newline
+  EXPECT_TRUE(probe("a,b,c\n1,2,3").is_csv); // no trailing newline
+  EXPECT_TRUE(probe("a,b\r1,2\r").is_csv);
   EXPECT_TRUE(probe("a,b\r\n1,2\r\n").is_csv);      // crlf
   EXPECT_TRUE(probe("a,b\n\"x,y\",2\n").is_csv);    // a quoted separator
   EXPECT_TRUE(probe("a,b\n\"x\ny\",2\n").is_csv);   // a quoted newline
@@ -183,6 +185,10 @@ TEST(CsvProbe, excel_declares_its_separator) {
 }
 
 TEST(RecordReader, parses_rfc4180) {
+  EXPECT_EQ(records("a,b\r1,2\r", {}),
+            (std::vector<std::vector<std::string>>{{"a", "b"}, {"1", "2"}}));
+  EXPECT_EQ(records("\r\n\r\"a\rb\",c\r\n", {}),
+            (std::vector<std::vector<std::string>>{{"a\rb", "c"}}));
   EXPECT_EQ(records("a,b\n1,2\n", {}),
             (std::vector<std::vector<std::string>>{{"a", "b"}, {"1", "2"}}));
   EXPECT_EQ(records("\"x,y\",2\n", {}),
@@ -258,6 +264,10 @@ TEST(CsvOptions, a_declared_separator_makes_anything_readable) {
 }
 
 TEST(CsvOptions, an_incoherent_dialect_is_a_caller_mistake) {
+  EXPECT_THROW(
+      (void)open(File::from_memory("a,b\n"),
+                 DecodeOptions::as_csv({.separator = ',', .quote = '\r'})),
+      std::invalid_argument);
   EXPECT_THROW(
       (void)open(File::from_memory("a,b\n"),
                  DecodeOptions::as_csv({.separator = '"', .quote = '"'}))
@@ -522,6 +532,8 @@ std::string edited(
 } // namespace
 
 TEST(CsvDocument, an_unedited_file_saves_as_it_reads) {
+  EXPECT_EQ(edited("\"a\nb\",c\r\n1,2\r\n", ""), "\"a\nb\",c\r\n1,2\r\n");
+  EXPECT_EQ(edited("a,b\r1,2\r", ""), "a,b\r1,2\r");
   EXPECT_EQ(edited("a,b\n1,2\n", ""), "a,b\n1,2\n");
   EXPECT_EQ(edited("a,b\r\n1,2", ""), "a,b\r\n1,2");
   EXPECT_EQ(edited("a\nb,c,d\n", ""), "a\nb,c,d\n");
@@ -577,6 +589,9 @@ TEST(CsvDocument, a_written_number_types_its_column_again) {
 }
 
 TEST(CsvDocument, the_separator_directive_is_kept) {
+  EXPECT_EQ(edited("sep=;\ra;b\r1;2\r", "",
+                   DecodeOptions::as_csv({.separator = ';'})),
+            "sep=;\ra;b\r1;2\r");
   EXPECT_EQ(edited("sep=;\na;b\n1;2\n", "",
                    DecodeOptions::as(FileType::comma_separated_values)),
             "sep=;\na;b\n1;2\n");
@@ -681,3 +696,28 @@ TEST(CsvDocument, a_column_edit_keeps_the_numbers_of_a_column) {
   EXPECT_EQ(sheet.cell(0, 1).value_type(), ValueType::float_number);
   EXPECT_EQ(sheet.dimensions().columns, 1);
 }
+
+TEST(CsvDocument, coordinates_and_growth_cannot_wrap) {
+  const Document document = open(File::from_memory("a,b\n1,2\n"),
+                                 DecodeOptions::as_csv({.separator = ','}))
+                                .as_csv_file()
+                                .document();
+  const Sheet sheet = document.root_element().first_child().as_sheet();
+  constexpr auto max = std::numeric_limits<std::uint32_t>::max();
+  constexpr std::uint32_t columns = std::uint32_t{1} << 24;
+  EXPECT_THROW((void)sheet.cell(columns, 0), std::out_of_range);
+  EXPECT_THROW(sheet.set_cell(columns, 0, CellValue("x")), std::out_of_range);
+  EXPECT_THROW(sheet.set_cell(0, max, CellValue("x")), std::out_of_range);
+  EXPECT_THROW(sheet.insert_rows(0, max), std::length_error);
+  EXPECT_THROW(sheet.insert_columns(0, columns), std::length_error);
+  EXPECT_EQ(sheet.dimensions().rows, 2);
+  EXPECT_EQ(sheet.dimensions().columns, 2);
+  EXPECT_EQ(sheet.cell(0, 0).value().text(), "a");
+  EXPECT_EQ(document
+                .element_by_id(sheet.cell(0, 0).identifier() |
+                               (std::uint64_t{1} << 56))
+                .type(),
+            ElementType::none);
+  sheet.delete_rows(1, max);
+  EXPECT_EQ(sheet.dimensions().rows, 1);
+}