Added StringViewStream to enable less copying when parsing string views (#303)

vincentlaucsb · github-advanced-security[bot] · web-flow · commit 539b6df67248 · 2026-04-15T01:03:17.000-07:00
* Added StringViewStream

 * Added opt-in footgun
 * Added opt-in safety switch for StreamParser path

* Fix single header issue

* Updated comments on CSVReader ownership

* Reverted parse(). Added parse_unsafe() for non-owning SVs.

* Potential fix for pull request finding 'CodeQL / Large object passed by value'

Co-authored-by: Copilot Autofix powered by AI &lt;62310815+github-advanced-security[bot]@users.noreply.github.com&gt;

---------

Co-authored-by: Copilot Autofix powered by AI &lt;62310815+github-advanced-security[bot]@users.noreply.github.com&gt;
diff --git a/AGENTS.md b/AGENTS.md
@@ -74,5 +74,7 @@ ThreadSafeDeque<CSVRow>
 4. **Don't ignore async:** Worker thread means exceptions must use `exception_ptr`.
 5. **Don't change one constructor:** Likely affects both mmap and stream paths.
 6. **Don't delete or simplify comments** unless they are trivially obvious (e.g. `// increment i`) or factually incorrect. Comments in this codebase frequently encode concurrency invariants, non-obvious design decisions, and hard-won bug context that cannot be recovered from the code alone.
+7. **Don't reference internal functions in public API comments.** Public API docs should describe user-visible behavior and contracts; internal helper/function details belong in internal docs.
+8. **`CSVReader` is non-copyable and non-movable.** The preferred idiom for sharing or transferring a reader is `std::unique_ptr<CSVReader>`. Document this at any API surface that might tempt callers to copy or move.
 
 See `tests/AGENTS.md` for test strategy, checklist, and conventions.
diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md
@@ -15,4 +15,5 @@ Operational/testing guidance:
 Notes:
 - Internal architecture content lives under include/internal to stay close to implementation.
 - Queue synchronization details are maintained only in THREADSAFE_DEQUE_DESIGN.md to avoid duplication.
+- Public API comments should remain user-facing and avoid references to internal helper/function details.
 
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -36,6 +36,8 @@
 - Exceptions from worker thread need `exception_ptr`
 - Changes to one constructor likely affect both paths
 - **Do not delete or simplify comments** unless trivially obvious or factually wrong — comments encode concurrency invariants and bug history
+- **Do not reference internal functions in public API comments** — public API docs should remain user-facing; internal details belong in internal docs
+- **`CSVReader` is non-copyable and non-movable** — the preferred sharing/transfer idiom is `std::unique_ptr<CSVReader>`
 
 ## Tests
 See `tests/AGENTS.md` for full test strategy, checklist, and conventions.
diff --git a/include/csv.hpp b/include/csv.hpp
@@ -1,5 +1,5 @@
 /*
-CSV for C++, version 3.2.0
+CSV for C++, version 3.3.0
 https://github.com/vincentlaucsb/csv-parser
 
 MIT License
diff --git a/include/internal/CMakeLists.txt b/include/internal/CMakeLists.txt
@@ -25,8 +25,9 @@ target_sources(csv
 		raw_csv_data.cpp
 		row_deque.hpp
 		single_thread_deque.hpp
+		string_view_stream.hpp
 		thread_safe_deque.hpp
-		)
+	)
 
 set_target_properties(csv PROPERTIES LINKER_LANGUAGE CXX)
 
diff --git a/include/internal/csv_reader.cpp b/include/internal/csv_reader.cpp
@@ -166,12 +166,15 @@ namespace csv {
      */
 	CSV_INLINE CSVReader::CSVReader(csv::string_view filename, CSVFormat format) : _format(format) {
 #if defined(__EMSCRIPTEN__)
-    this->owned_file_stream = std::unique_ptr<std::ifstream>(new std::ifstream(std::string(filename), std::ios::binary));
-    if (!this->owned_file_stream->is_open()) {
+    this->owned_stream = std::unique_ptr<std::istream>(
+        new std::ifstream(std::string(filename), std::ios::binary)
+    );
+
+    if (!(*this->owned_stream)) {
         throw std::runtime_error("Cannot open file " + std::string(filename));
     }
 
-    this->init_from_stream(*this->owned_file_stream, format);
+    this->init_from_stream(*this->owned_stream, format);
 #else
     auto head = internals::get_csv_head(filename);
         using Parser = internals::MmapParser;
diff --git a/include/internal/csv_reader.hpp b/include/internal/csv_reader.hpp
@@ -75,6 +75,14 @@ namespace csv {
      *  All rows are compared to the column names for length consistency
      *  - By default, rows that are too short or too long are dropped
      *  - Custom behavior can be defined by overriding bad_row_handler in a subclass
+     *
+     *  **Ownership and sharing:** CSVReader is neither copyable nor movable because it
+     *  manages a live parsing state (worker thread, internal queue, and an optional stream
+     *  reference). To share or transfer a reader, wrap it in a `std::unique_ptr<CSVReader>`:
+     *  @code{.cpp}
+     *  auto reader = std::make_unique<csv::CSVReader>("data.csv");
+     *  process(std::move(reader));   // transfer ownership
+     *  @endcode
      */
     class CSVReader {
     public:
@@ -186,6 +194,20 @@ namespace csv {
         CSVReader(TStream &source, CSVFormat format = CSVFormat::guess_csv()) : _format(format) {
             this->init_from_stream(source, format);
         }
+
+        /** @brief Construct CSVReader from an owned std::istream
+         *
+         *  This is an opt-in safety switch for stream lifetime management.
+         *  CSVReader takes ownership and guarantees the stream outlives parsing.
+         */
+        CSVReader(std::unique_ptr<std::istream> source,
+            const CSVFormat& format = CSVFormat::guess_csv()) : _format(format), owned_stream(std::move(source)) {
+            if (!this->owned_stream) {
+                throw std::invalid_argument("CSVReader requires a non-null stream");
+            }
+
+            this->init_from_stream(*this->owned_stream, format);
+        }
         ///@}
 
         CSVReader(const CSVReader&) = delete;             ///< Not copyable
@@ -261,10 +283,12 @@ namespace csv {
         /** Queue of parsed CSV rows */
         std::unique_ptr<RowCollection> records{new RowCollection(100)};
 
-    #if defined(__EMSCRIPTEN__)
-        /** Owned file stream used by filename constructor fallback to stream parsing. */
-        std::unique_ptr<std::ifstream> owned_file_stream = nullptr;
-    #endif
+        /**
+         * Optional owned stream used by two paths:
+         *  1) Emscripten filename-constructor fallback to stream parsing
+         *  2) Opt-in ownership constructor taking std::unique_ptr<std::istream>
+         */
+        std::unique_ptr<std::istream> owned_stream = nullptr;
 
         size_t n_cols = 0;  /**< The number of columns in this CSV */
         size_t _n_rows = 0; /**< How many rows (minus header) have been read so far */
diff --git a/include/internal/csv_row.cpp b/include/internal/csv_row.cpp
@@ -8,7 +8,7 @@
 
 namespace csv {
     namespace internals {
-        csv::string_view get_trimmed(csv::string_view sv, const WhitespaceMap& ws_flags) noexcept
+        CSV_INLINE csv::string_view get_trimmed(csv::string_view sv, const WhitespaceMap& ws_flags) noexcept
         {
             // Lazy trim only when requested
             size_t start = 0;
diff --git a/include/internal/csv_utility.cpp b/include/internal/csv_utility.cpp
@@ -1,19 +1,40 @@
 #include <sstream>
 #include <vector>
+#include <memory>
 
 #include "csv_utility.hpp"
+#include "string_view_stream.hpp"
 
 namespace csv {
-    /** Shorthand function for parsing an in-memory CSV string
+    /** Shorthand function for parsing an in-memory CSV string.
+     *
+     *  Copies the input into an owned stringstream, so the caller's backing
+     *  memory may be freed immediately after this call returns.
      *
      *  @return A collection of CSVRow objects
      *
      *  @par Example
      *  @snippet tests/test_read_csv.cpp Parse Example
      */
     CSV_INLINE CSVReader parse(csv::string_view in, CSVFormat format) {
-        std::stringstream stream(std::string(in.data(), in.length()));
-        return CSVReader(stream, format);
+        std::unique_ptr<std::istream> ss(new std::stringstream(std::string(in)));
+        return CSVReader(std::move(ss), format);
+    }
+
+    /** Parse CSV from an in-memory view with zero copy.
+     *
+     *  Creates a non-owning stream adapter over the provided string_view.
+     *  The caller is responsible for keeping backing memory valid and immutable
+     *  while CSVReader may request additional rows.
+     *
+     *  Already materialized CSVRows remain safe because parsed chunk data is
+     *  owned by RawCSVData.
+     *
+     *  @return A collection of CSVRow objects
+     */
+    CSV_INLINE CSVReader parse_unsafe(csv::string_view in, CSVFormat format) {
+        std::unique_ptr<std::istream> stream(new internals::StringViewStream(in));
+        return CSVReader(std::move(stream), format);
     }
 
     /** Parses a CSV string with no headers
@@ -28,19 +49,28 @@ namespace csv {
     }
 
     /** Parse a RFC 4180 CSV string, returning a collection
-     *  of CSVRow objects
+     *  of CSVRow objects.
+     *
+     *  String literals have static storage duration, so the zero-copy path is
+     *  safe here.
      *
      *  @par Example
      *  @snippet tests/test_read_csv.cpp Escaped Comma
      *
      */
     CSV_INLINE CSVReader operator ""_csv(const char* in, size_t n) {
-        return parse(csv::string_view(in, n));
+        return parse_unsafe(csv::string_view(in, n));
     }
 
-    /** A shorthand for csv::parse_no_header() */
+    /** A shorthand for csv::parse_no_header().
+     *
+     *  String literals have static storage duration, so the zero-copy path is
+     *  safe here.
+     */
     CSV_INLINE CSVReader operator ""_csv_no_header(const char* in, size_t n) {
-        return parse_no_header(csv::string_view(in, n));
+        CSVFormat format;
+        format.header_row(-1);
+        return parse_unsafe(csv::string_view(in, n), format);
     }
 
     /**
diff --git a/include/internal/csv_utility.hpp b/include/internal/csv_utility.hpp
@@ -3,6 +3,7 @@
 #include "csv_format.hpp"
 #include "csv_reader.hpp"
 #include "data_type.hpp"
+#include "string_view_stream.hpp"
 
 #include <string>
 #include <type_traits>
@@ -19,12 +20,31 @@ namespace csv {
     };
 
     /** @name Shorthand Parsing Functions
-     *  @brief Convienience functions for parsing small strings
+     *  @brief Convenience functions for parsing small strings
      */
      ///@{
     CSVReader operator ""_csv(const char*, size_t);
     CSVReader operator ""_csv_no_header(const char*, size_t);
+
+    /** Parse CSV from a string view, copying the input into an owned buffer.
+     *
+     *  Safe for any string_view regardless of the caller's ownership of the
+     *  underlying memory.
+     */
     CSVReader parse(csv::string_view in, CSVFormat format = CSVFormat());
+
+    /** Parse CSV from an in-memory view with zero copy.
+     *
+     *  WARNING: Non-owning path. The caller must ensure `in`'s backing memory
+     *  remains valid and immutable while the reader may request additional rows
+     *  from the source stream.
+     *
+     *  Already materialized CSVRows remain safe because parsed chunk data is
+     *  owned by RawCSVData, so make sure you grab all the CSVRows you need
+     *  before the underlying string is destroyed.
+     */
+    CSVReader parse_unsafe(csv::string_view in, CSVFormat format = CSVFormat());
+
     CSVReader parse_no_header(csv::string_view in);
     ///@}
 
diff --git a/include/internal/string_view_stream.hpp b/include/internal/string_view_stream.hpp
@@ -0,0 +1,98 @@
+#pragma once
+
+#include <algorithm>
+#include <cstring>
+#include <istream>
+
+#include "common.hpp"
+
+namespace csv {
+    namespace internals {
+        /**
+         * streambuf adapter over csv::string_view with no data copy.
+         *
+         * The underlying memory must remain valid and immutable for the entire
+         * lifetime of this stream object.
+         */
+        class StringViewStreamBuf : public std::streambuf {
+        public:
+            explicit StringViewStreamBuf(csv::string_view view) {
+                char* begin = const_cast<char*>(view.data());
+                char* end = begin + view.size();
+                this->setg(begin, begin, end);
+            }
+
+        protected:
+            std::streamsize xsgetn(char* s, std::streamsize count) override {
+                const std::streamsize avail = this->egptr() - this->gptr();
+                const std::streamsize n = std::min(avail, count);
+                if (n > 0) {
+                    std::memcpy(s, this->gptr(), static_cast<size_t>(n));
+                    this->gbump(static_cast<int>(n));
+                }
+                return n;
+            }
+
+            pos_type seekoff(off_type off, std::ios_base::seekdir dir,
+                std::ios_base::openmode which = std::ios_base::in) override {
+                if (!(which & std::ios_base::in)) {
+                    return pos_type(off_type(-1));
+                }
+
+                const auto begin = this->eback();
+                const auto curr = this->gptr();
+                const auto end = this->egptr();
+
+                off_type base = 0;
+                if (dir == std::ios_base::beg) {
+                    base = 0;
+                }
+                else if (dir == std::ios_base::cur) {
+                    base = static_cast<off_type>(curr - begin);
+                }
+                else if (dir == std::ios_base::end) {
+                    base = static_cast<off_type>(end - begin);
+                }
+                else {
+                    return pos_type(off_type(-1));
+                }
+
+                const off_type next = base + off;
+                const off_type size = static_cast<off_type>(end - begin);
+                if (next < 0 || next > size) {
+                    return pos_type(off_type(-1));
+                }
+
+                this->setg(begin, begin + next, end);
+                return pos_type(next);
+            }
+
+            pos_type seekpos(pos_type pos,
+                std::ios_base::openmode which = std::ios_base::in) override {
+                return this->seekoff(off_type(pos), std::ios_base::beg, which);
+            }
+        };
+
+        /**
+         * Lightweight istream over csv::string_view with zero copy.
+         *
+         * WARNING: The caller is responsible for ensuring the backing memory
+         * outlives this stream and any CSVReader parsing it.
+         */
+        class StringViewStream : public std::istream {
+        public:
+            explicit StringViewStream(csv::string_view view)
+                : std::istream(nullptr), _buf(view) {
+                this->rdbuf(&_buf);
+            }
+
+            StringViewStream(const StringViewStream&) = delete;
+            StringViewStream(StringViewStream&&) = delete;
+            StringViewStream& operator=(const StringViewStream&) = delete;
+            StringViewStream& operator=(StringViewStream&&) = delete;
+
+        private:
+            StringViewStreamBuf _buf;
+        };
+    }
+}
diff --git a/tests/test_edge_cases_large_rows.cpp b/tests/test_edge_cases_large_rows.cpp
@@ -239,4 +239,37 @@ TEST_CASE("Issue #218 - Infinite read loop detection", "[issue_218]") {
         );
     }
 }
+
+// Verify parse_unsafe() (non-owning StringViewStream path) delivers correct values
+// across the 10MB chunk boundary. Distinct per-column values (i*5+col) ensure that
+// field corruption or mis-alignment at a chunk transition would be detected.
+TEST_CASE("parse_unsafe() chunk boundary integrity", "[parse_unsafe_chunk_boundary]") {
+    const size_t n_rows = 500000;
+
+    std::string csv_data;
+    csv_data.reserve(n_rows * 35);
+    csv_data += "A,B,C,D,E\r\n";
+    for (size_t i = 0; i < n_rows; i++) {
+        csv_data += std::to_string(i * 5 + 0) + ','
+                  + std::to_string(i * 5 + 1) + ','
+                  + std::to_string(i * 5 + 2) + ','
+                  + std::to_string(i * 5 + 3) + ','
+                  + std::to_string(i * 5 + 4) + "\r\n";
+    }
+
+    // csv_data outlives the reader — the non-owning path is safe here.
+    auto reader = parse_unsafe(csv_data);
+
+    REQUIRE(reader.get_col_names() == std::vector<std::string>({"A", "B", "C", "D", "E"}));
+
+    size_t i = 0;
+    for (auto& row : reader) {
+        REQUIRE(row.size() == 5);
+        for (size_t col = 0; col < 5; col++) {
+            REQUIRE(row[col].get<size_t>() == i * 5 + col);
+        }
+        i++;
+    }
+    REQUIRE(i == n_rows);
+}
 #endif

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@`
`8`	`8`
`9`	`9`	`namespace csv {`
`10`	`10`	`namespace internals {`
`11`		`- csv::string_view get_trimmed(csv::string_view sv, const WhitespaceMap& ws_flags) noexcept`
	`11`	`+ CSV_INLINE csv::string_view get_trimmed(csv::string_view sv, const WhitespaceMap& ws_flags) noexcept`
`12`	`12`	`{`
`13`	`13`	`// Lazy trim only when requested`
`14`	`14`	`size_t start = 0;`