Skip to content

Commit

Permalink
Fix bad conversion of array-of-strings to string. (#817)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jtv committed Apr 7, 2024
1 parent edad18b commit 39d4ce0
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 1 deletion.
1 change: 1 addition & 0 deletions NEWS
Original file line number Diff line number Diff line change
@@ -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
Expand Down
7 changes: 6 additions & 1 deletion include/pqxx/internal/conversions.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
});
}
Expand Down
20 changes: 20 additions & 0 deletions test/unit/test_array.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
16 changes: 16 additions & 0 deletions test/unit/test_stream_to.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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

0 comments on commit 39d4ce0

Please sign in to comment.