Bugfix: (sparse) arrays' size_buffer() - not accounting for separator…

… for NULL values (#921)

Fix string conversion buffer budget for arrays containing null values.

include/pqxx/internal/conversions.hxx

@@ -1120,8 +1120,11 @@ public:
                      // Budget for each element includes a terminating zero.
                      // We won't actually be wanting those, but don't subtract
                      // that one byte: we want room for a separator instead.
+                     // However, std::size(s_null) doesn't account for the
+                     // terminating zero, so add one to make s_null pay for its
+                     // own separator.
                      return acc + (pqxx::is_null(elt) ?
-                                     std::size(s_null) :
+                                     (std::size(s_null) + 1) :
                                      elt_traits::size_buffer(elt));
                    });
     else
@@ -1130,9 +1133,11 @@ public:
                    [](std::size_t acc, elt_type const &elt) {
                      // Opening and closing quotes, plus worst-case escaping,
                      // and the one byte for the trailing zero becomes room
-                     // for a separator.
+                     // for a separator. However, std::size(s_null) doesn't
+                     // account for the terminating zero, so add one to make
+                     // s_null pay for its own separator.
                      std::size_t const elt_size{
-                       pqxx::is_null(elt) ? std::size(s_null) :
+                       pqxx::is_null(elt) ? (std::size(s_null) + 1) :
                                             elt_traits::size_buffer(elt)};
                      return acc + 2 * elt_size + 2;
                    });

test/unit/test_array.cxx

@@ -436,6 +436,48 @@ void test_array_generate_empty_strings()
     "Array of 12 empty strings came out wrong.");
 }
 
+void test_sparse_arrays()
+{
+  // Reproduce #922 : NULL not paying for its separator in an array, causing
+  // problems in sparse arrays.
+
+  // If NULL didn't pay for its separator, the size allocated for an array-like
+  // object filled with null-like values would be too small.
+
+  auto arrayOfNulls = std::vector<std::optional<int>>(4, std::nullopt);
+  std::string arrayOfNullsStr = "{NULL,NULL,NULL,NULL}";
+
+  PQXX_CHECK(
+    pqxx::size_buffer(arrayOfNulls) >= arrayOfNullsStr.size(),
+    "Buffer size allocated for an array of optional<int> filled with nulls "
+    "was too small.");
+
+  PQXX_CHECK_EQUAL(
+    pqxx::to_string(arrayOfNulls), arrayOfNullsStr,
+    "Array of optional<int> filled with std::nullopt came out wrong. ");
+
+
+  // If the array-like object is instead sparsely-filled, the values contained
+  // in non-null elements may leave behind excess unused size which hides the
+  // problem better - it only becomes an error once the array contains enough
+  // null-like values to outweigh this excess unused size.
+
+  std::array<std::optional<int>, 14> sparseArray;
+  sparseArray[sparseArray.size() - 1] = 42;
+
+  std::string sparseArrayStr =
+    "{NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,NULL,"
+    "42}";
+
+  PQXX_CHECK(
+    pqxx::size_buffer(sparseArray) >= sparseArrayStr.size(),
+    "Buffer size allocated for a sparsely-filled array of optional<int> was "
+    "too small.");
+
+  PQXX_CHECK_EQUAL(
+    pqxx::to_string(sparseArray), sparseArrayStr,
+    "A sparsely-filled array of optional<int> came out wrong.");
+}
 
 void test_array_generate()
 {
@@ -445,6 +487,7 @@ void test_array_generate()
   test_generate_multiple_items();
   test_generate_nested_array();
   test_generate_escaped_strings();
+  test_sparse_arrays();
 }