Stream rows ending in empty string properly. (#777)

Fixes #774.

The streaming code assumed that a line of `COPY` data will never end in
an empty line.

NEWS

@@ -1,4 +1,5 @@
 7.9.0
+ - Fix assertion failure when streaming row ending in empty string. (#774)
  - Support parameterised versions of `query()` etc. (#646)
  - Put all feature tests back in config header. (#732)
  - Automate integration of feature tests into both CMake & autoconf. (#747)

config/Makefile.in

@@ -124,7 +124,7 @@ am__can_run_installinfo = \
   esac
 am__tagged_files = $(HEADERS) $(SOURCES) $(TAGS_FILES) $(LISP)
 am__DIST_COMMON = $(srcdir)/Makefile.in compile config.guess \
-	config.sub install-sh ltmain.sh missing mkinstalldirs
+	config.sub depcomp install-sh ltmain.sh missing mkinstalldirs
 DISTFILES = $(DIST_COMMON) $(DIST_SOURCES) $(TEXINFOS) $(EXTRA_DIST)
 ACLOCAL = @ACLOCAL@
 AMTAR = @AMTAR@

configure

@@ -4108,11 +4108,11 @@ if test x$ac_prog_cxx_stdcxx = xno
 then :
   { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for $CXX option to enable C++11 features" >&5
 printf %s "checking for $CXX option to enable C++11 features... " >&6; }
-if test ${ac_cv_prog_cxx_cxx11+y}
+if test ${ac_cv_prog_cxx_11+y}
 then :
   printf %s "(cached) " >&6
 else $as_nop
-  ac_cv_prog_cxx_cxx11=no
+  ac_cv_prog_cxx_11=no
 ac_save_CXX=$CXX
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
@@ -4154,11 +4154,11 @@ if test x$ac_prog_cxx_stdcxx = xno
 then :
   { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for $CXX option to enable C++98 features" >&5
 printf %s "checking for $CXX option to enable C++98 features... " >&6; }
-if test ${ac_cv_prog_cxx_cxx98+y}
+if test ${ac_cv_prog_cxx_98+y}
 then :
   printf %s "(cached) " >&6
 else $as_nop
-  ac_cv_prog_cxx_cxx98=no
+  ac_cv_prog_cxx_98=no
 ac_save_CXX=$CXX
 cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */

include/pqxx/connection.hxx

@@ -1098,6 +1098,12 @@ private:
   void PQXX_PRIVATE unregister_transaction(transaction_base *) noexcept;
 
   friend struct internal::gate::connection_stream_from;
+  /// Read a line of COPY output.
+  /** If the output indicates that the COPY has ended, the buffer pointer
+   * will be null and the size will be zero.  Otherwise, the pointer will hold
+   * a buffer containing the line, and size will be its length not including
+   * the newline at the end.
+   */
   std::pair<std::unique_ptr<char, void (*)(void const *)>, std::size_t>
   read_copy_line();
 

include/pqxx/doc/streams.md

@@ -76,7 +76,8 @@ the value types you ask for:
 ```
 
 On each iteration, the stream gives you a `std::tuple` of the column types you
-specify.  It converts the row's fields (which internally arrive at the client in text format) to your chosen types.
+specify.  It converts the row's fields (which internally arrive at the client
+in text format) to your chosen types.
 
 The `auto [name, score]` in the example is a _structured binding_ which unpacks
 the tuple's fields into separate variables.  If you prefer, you can choose to

include/pqxx/internal/stream_query.hxx

@@ -174,7 +174,7 @@ private:
     auto const line_size{std::size(line)};
 #endif
 
-    assert(offset < line_size);
+    assert(offset <= line_size);
 
     char const *lp{std::data(line)};
 
@@ -185,10 +185,10 @@ private:
 
     if ((lp[offset] == '\\') and (lp[offset + 1] == 'N'))
     {
-      // Null field.
-      assert(lp[offset + 2] == '\t');
-      // Consume the "\N" and the field separator.
+      // Null field.  Consume the "\N" and the field separator.
       offset += 3;
+      assert(offset <= (line_size + 1));
+      assert(lp[offset - 1] == '\t');
       // Return a null value.  There's nothing to write into m_row.
       return {offset, write, {}};
     }
@@ -267,7 +267,7 @@ private:
     using field_type = strip_t<TARGET>;
     using nullity = nullness<field_type>;
 
-    assert(offset < std::size(line));
+    assert(offset <= std::size(line));
 
     auto [new_offset, new_write, text]{read_field(line, offset, write)};
     offset = new_offset;

include/pqxx/internal/stream_query_impl.hxx

@@ -105,12 +105,15 @@ private:
   bool done() const noexcept { return m_home->done(); }
 
   /// Read a line from the stream, store it in the iterator.
+  /** Replaces the newline at the end with a tab, as a sentinel to simplify
+   * (and thus hopefully speed up) the field parsing loop.
+   */
   void consume_line() &
   {
     auto [line, size]{m_home->read_line()};
     m_line = std::move(line);
     m_line_size = size;
-    if (size > 0)
+    if (m_line)
     {
       // We know how many fields to expect.  Replace the newline at the end
       // with the field separator, so the parsing loop only needs to scan for a

test/unit/test_stream_query.cxx

@@ -192,6 +192,36 @@ void test_stream_handles_nulls_in_all_places()
 }
 
 
+void test_stream_handles_empty_string()
+{
+  pqxx::connection conn;
+  pqxx::work tx{conn};
+
+  std::string out{"<uninitialised>"};
+  for (auto [empty] : tx.stream<std::string_view>("SELECT ''"))
+    out = empty;
+  PQXX_CHECK_EQUAL(out, "", "Empty string_view parsed wrong.");
+
+  out = "<uniniutialised>";
+  int num{0};
+  for (auto [i, s] : tx.stream<int, std::string_view>("SELECT 99, ''"))
+  {
+    num = i;
+    out = s;
+  }
+  PQXX_CHECK_EQUAL(num, 99, "Integer came out wrong before empty string.");
+  PQXX_CHECK_EQUAL(out, "", "Final empty string came out wrong.");
+
+  for (auto [s2, i2] : tx.stream<std::string_view, int>("SELECT '', 33"))
+  {
+    out = s2;
+    num = i2;
+  }
+  PQXX_CHECK_EQUAL(out, "", "Leading empty string came out wrong.");
+  PQXX_CHECK_EQUAL(num, 33, "Integer came out wrong after empty string.");
+}
+
+
 PQXX_REGISTER_TEST(test_stream_handles_empty);
 PQXX_REGISTER_TEST(test_stream_does_escaping);
 PQXX_REGISTER_TEST(test_stream_reads_simple_values);
@@ -200,4 +230,5 @@ PQXX_REGISTER_TEST(test_stream_iterates);
 PQXX_REGISTER_TEST(test_stream_reads_nulls_as_optionals);
 PQXX_REGISTER_TEST(test_stream_parses_awkward_strings);
 PQXX_REGISTER_TEST(test_stream_handles_nulls_in_all_places);
+PQXX_REGISTER_TEST(test_stream_handles_empty_string);
 } // namespace

test/unit/test_string_conversion.cxx

@@ -188,6 +188,7 @@ void test_string_view_conversion()
   PQXX_CHECK(
     std::begin(buf) < end and end < std::end(buf),
     "string_view into_buf did not stay within its buffer.");
+  assert(end > std::begin(buf));
   PQXX_CHECK(
     *(end - 1) == '\0', "string_view into_buf did not zero-terminate.");
   PQXX_CHECK_EQUAL(