From 39d4ce0f1e93cde9d2283d6aa82c3cf2385281b0 Mon Sep 17 00:00:00 2001
From: Jeroen Vermeulen <jtv@users.noreply.github.com>
Date: Sun, 7 Apr 2024 21:51:24 +0200
Subject: [PATCH] Fix bad conversion of array-of-strings to string. (#817)

Fixes: #816.

The generic conversion to an SQL array computed its budget just slightly
too tightly, which only became obvious when converting an array of at
least 4 empty strings.
---
 NEWS                                  |  1 +
 include/pqxx/internal/conversions.hxx |  7 ++++++-
 test/unit/test_array.cxx              | 20 ++++++++++++++++++++
 test/unit/test_stream_to.cxx          | 16 ++++++++++++++++
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index e66a1a4b1..ab69a363d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,5 @@
 7.9.1
+ - Fix bad conversion of array of empty strings to string. (#816)
  - Move `[[likely]]` feature check back to compile time, to speed up configure.
  - Support `[[assume(...)]]`.
 7.9.0
diff --git a/include/pqxx/internal/conversions.hxx b/include/pqxx/internal/conversions.hxx
index 3b086bdcb..1c05f48cb 100644
--- a/include/pqxx/internal/conversions.hxx
+++ b/include/pqxx/internal/conversions.hxx
@@ -1025,6 +1025,7 @@ public:
 
   static char *into_buf(char *begin, char *end, Container const &value)
   {
+    assert(begin <= end);
     std::size_t const budget{size_buffer(value)};
     if (internal::cmp_less(end - begin, budget))
       throw conversion_overrun{
@@ -1059,8 +1060,12 @@ public:
         // Use the tail end of the destination buffer as an intermediate
         // buffer.
         auto const elt_budget{pqxx::size_buffer(elt)};
+	assert(elt_budget < static_cast<std::size_t>(end - here));
         for (char const c : elt_traits::to_buf(end - elt_budget, end, elt))
         {
+	  // We copy the intermediate buffer into the final buffer, char by
+	  // char, with escaping where necessary.
+	  // TODO: This will not work for all encodings.  UTF8 & ASCII are OK.
           if (c == '\\' or c == '"')
             *here++ = '\\';
           *here++ = c;
@@ -1101,7 +1106,7 @@ public:
                      // but don't count the trailing zeroes.
                      std::size_t const elt_size{
                        pqxx::is_null(elt) ? std::size(s_null) :
-                                            elt_traits::size_buffer(elt) - 1};
+                                            elt_traits::size_buffer(elt)};
                      return acc + 2 * elt_size + 2;
                    });
   }
diff --git a/test/unit/test_array.cxx b/test/unit/test_array.cxx
index 28636132d..c1b465ef2 100644
--- a/test/unit/test_array.cxx
+++ b/test/unit/test_array.cxx
@@ -420,6 +420,25 @@ void test_generate_escaped_strings()
 }
 
 
+void test_array_generate_empty_strings()
+{
+  // Reproduce #816: Under-budgeted conversion of empty strings in arrays.
+  PQXX_CHECK_EQUAL(
+    pqxx::to_string(std::vector<std::string>({""})),
+    "{\"\"}",
+    "Array of one empty string came out wrong.");
+  PQXX_CHECK_EQUAL(
+    pqxx::to_string(std::vector<std::string>({"", "", "", ""})),
+    "{\"\",\"\",\"\",\"\"}",
+    "Array of 4 empty strings came out wrong.");
+  PQXX_CHECK_EQUAL(
+    pqxx::to_string(std::vector<std::string>(
+        {"", "", "", "", "", "", "", "", "", "", "", ""})),
+    "{\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\",\"\"}",
+    "Array of 12 empty strings came out wrong.");
+}
+
+
 void test_array_generate()
 {
   test_generate_empty_array();
@@ -696,4 +715,5 @@ PQXX_REGISTER_TEST(test_array_parses_quoted_strings);
 PQXX_REGISTER_TEST(test_array_parses_multidim_arrays);
 PQXX_REGISTER_TEST(test_array_at_checks_bounds);
 PQXX_REGISTER_TEST(test_array_iterates_in_row_major_order);
+PQXX_REGISTER_TEST(test_array_generate_empty_strings);
 } // namespace
diff --git a/test/unit/test_stream_to.cxx b/test/unit/test_stream_to.cxx
index d751658b2..fb21201f3 100644
--- a/test/unit/test_stream_to.cxx
+++ b/test/unit/test_stream_to.cxx
@@ -544,6 +544,21 @@ void test_stream_to_moves_into_optional()
 }
 
 
+void test_stream_to_empty_strings()
+{
+  // Reproduce #816: Streaming an array of 4 or more empty strings to a table
+  // using stream_to crashes.
+  pqxx::connection cx;
+  pqxx::transaction tx{cx};
+  tx.exec0("CREATE TEMP TABLE strs (list text[])");
+  std::vector<std::string> empties{"", "", "", ""};
+  auto stream{pqxx::stream_to::table(tx, {"strs"})};
+  stream.write_values(std::variant<std::vector<std::string>>{empties});
+  stream.complete();
+  tx.commit();
+}
+
+
 PQXX_REGISTER_TEST(test_stream_to);
 PQXX_REGISTER_TEST(test_container_stream_to);
 PQXX_REGISTER_TEST(test_stream_to_does_nonnull_optional);
@@ -553,4 +568,5 @@ PQXX_REGISTER_TEST(test_stream_to_quotes_arguments);
 PQXX_REGISTER_TEST(test_stream_to_optionals);
 PQXX_REGISTER_TEST(test_stream_to_escaping);
 PQXX_REGISTER_TEST(test_stream_to_moves_into_optional);
+PQXX_REGISTER_TEST(test_stream_to_empty_strings);
 } // namespace