Skip to content

Commit 61d6b86

Browse files
kevinAlbseramongodb
andcommitted
CDRIVER-5733 fix behavior of bson_string_truncate (#1736)
* test behavior of `bson_truncate` * support truncating above `INT_MAX` * avoid realloc for truncation of same size * note realloc uses power-of-two boundary * document possible abort This may occur if `bson_string_truncate` is called with UINT32_MAX. * reword size limit warnings --------- Co-authored-by: Ezra Chung <[email protected]>
1 parent bc3a894 commit 61d6b86

File tree

8 files changed

+83
-18
lines changed

8 files changed

+83
-18
lines changed

src/libbson/doc/bson_string_append.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ Description
2222

2323
Appends the ASCII or UTF-8 encoded string ``str`` to ``string``. This is not suitable for embedding NULLs in strings.
2424

25-
.. warning:: This function will abort if the length of the resulting string (including the NULL terminator) would exceed ``UINT32_MAX``.
25+
.. warning:: The length of the resulting string (including the ``NULL`` terminator) MUST NOT exceed ``UINT32_MAX``.

src/libbson/doc/bson_string_append_c.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ Description
2222

2323
Appends ``c`` to the string builder ``string``.
2424

25-
.. warning:: This function will abort if the length of the resulting string (including the NULL terminator) would exceed ``UINT32_MAX``.
25+
.. warning:: The length of the resulting string (including the ``NULL`` terminator) MUST NOT exceed ``UINT32_MAX``.

src/libbson/doc/bson_string_append_printf.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ Description
2323

2424
Like bson_string_append() but formats a printf style string and then appends that to ``string``.
2525

26-
.. warning:: This function will abort if the length of the resulting string (including the NULL terminator) would exceed ``UINT32_MAX``.
26+
.. warning:: The length of the resulting string (including the ``NULL`` terminator) MUST NOT exceed ``UINT32_MAX``.

src/libbson/doc/bson_string_append_unichar.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ Description
2222

2323
Appends a unicode character to string. The character will be encoded into its multi-byte UTF-8 representation.
2424

25-
.. warning:: This function will abort if the length of the resulting string (including the NULL terminator) would exceed ``UINT32_MAX``.
25+
.. warning:: The length of the resulting string (including the ``NULL`` terminator) MUST NOT exceed ``UINT32_MAX``.

src/libbson/doc/bson_string_new.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ Description
2121

2222
Creates a new string builder, which uses power-of-two growth of buffers. Use the various bson_string_append*() functions to append to the string.
2323

24-
.. warning:: This function will abort if the length of the resulting string (including the NULL terminator) would exceed ``UINT32_MAX``.
24+
.. warning:: The length of the resulting string (including the ``NULL`` terminator) MUST NOT exceed ``UINT32_MAX``.
2525

2626
Returns
2727
-------

src/libbson/doc/bson_string_truncate.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,4 @@ Truncates the string so that it is ``len`` bytes in length. This must be smaller
2424

2525
A ``\0`` byte will be placed where the end of the string occurs.
2626

27+
.. warning:: The length of the resulting string (including the ``NULL`` terminator) MUST NOT exceed ``UINT32_MAX``.

src/libbson/src/bson/bson-string.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ bson_string_append_printf (bson_string_t *string, const char *format, ...)
292292
* Truncate the string @string to @len bytes.
293293
*
294294
* The underlying memory will be released via realloc() down to
295-
* the minimum required size specified by @len.
295+
* the minimum required size (at power-of-two boundary) specified by @len.
296296
*
297297
* Returns:
298298
* None.
@@ -307,19 +307,17 @@ void
307307
bson_string_truncate (bson_string_t *string, /* IN */
308308
uint32_t len) /* IN */
309309
{
310-
uint32_t alloc;
311-
312-
BSON_ASSERT (string);
313-
BSON_ASSERT (len < INT_MAX);
314-
315-
alloc = len + 1;
316-
317-
if (alloc < 16) {
318-
alloc = 16;
310+
BSON_ASSERT_PARAM (string);
311+
if (len == string->len) {
312+
return;
319313
}
320-
321-
if (!bson_is_power_of_two (alloc)) {
322-
alloc = (uint32_t) bson_next_power_of_two ((size_t) alloc);
314+
uint32_t needed = len;
315+
BSON_ASSERT (needed < UINT32_MAX);
316+
needed += 1u; // Add one for trailing NULL byte.
317+
uint32_t alloc = bson_next_power_of_two_u32 (needed);
318+
if (alloc == 0) {
319+
// Overflowed: saturate at UINT32_MAX.
320+
alloc = UINT32_MAX;
323321
}
324322

325323
string->str = bson_realloc (string->str, alloc);

src/libbson/tests/test-string.c

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,61 @@ test_bson_strcasecmp (void)
304304
BSON_ASSERT (bson_strcasecmp ("FoZ", "foo") > 0);
305305
}
306306

307+
static void
308+
test_bson_string_truncate (void)
309+
{
310+
// Test truncating.
311+
{
312+
bson_string_t *str = bson_string_new ("foobar");
313+
ASSERT_CMPSIZE_T (str->len, ==, 6u);
314+
ASSERT_CMPSIZE_T (str->alloc, ==, 8u);
315+
316+
bson_string_truncate (str, 2);
317+
ASSERT_CMPSTR (str->str, "fo");
318+
ASSERT_CMPSIZE_T (str->len, ==, 2u);
319+
ASSERT_CMPSIZE_T (str->alloc, ==, 4u);
320+
bson_string_free (str, true);
321+
}
322+
323+
// Test truncating to same length.
324+
{
325+
bson_string_t *str = bson_string_new ("foobar");
326+
ASSERT_CMPSIZE_T (str->len, ==, 6u);
327+
ASSERT_CMPSIZE_T (str->alloc, ==, 8u);
328+
329+
bson_string_truncate (str, 6u);
330+
ASSERT_CMPSTR (str->str, "foobar");
331+
ASSERT_CMPUINT32 (str->len, ==, 6u);
332+
ASSERT_CMPSIZE_T (str->alloc, ==, 8u);
333+
bson_string_free (str, true);
334+
}
335+
336+
// Test truncating to 0.
337+
{
338+
bson_string_t *str = bson_string_new ("foobar");
339+
ASSERT_CMPSIZE_T (str->len, ==, 6u);
340+
ASSERT_CMPSIZE_T (str->alloc, ==, 8u);
341+
342+
bson_string_truncate (str, 0u);
343+
ASSERT_CMPSTR (str->str, "");
344+
ASSERT_CMPUINT32 (str->len, ==, 0u);
345+
ASSERT_CMPSIZE_T (str->alloc, ==, 1u);
346+
bson_string_free (str, true);
347+
}
348+
349+
// Test truncating beyond string length.
350+
// The documentation for `bson_string_truncate` says the truncated length "must be smaller or equal to the current
351+
// length of the string.". However, `bson_string_truncate` does not reject greater lengths. For backwards
352+
// compatibility, this behavior is preserved.
353+
{
354+
bson_string_t *str = bson_string_new ("a");
355+
bson_string_truncate (str, 2u); // Is not rejected.
356+
ASSERT_CMPUINT32 (str->len, ==, 2u);
357+
ASSERT_CMPUINT32 (str->alloc, ==, 4u);
358+
bson_string_free (str, true);
359+
}
360+
}
361+
307362
static void
308363
test_bson_string_capacity (void *unused)
309364
{
@@ -356,6 +411,16 @@ test_bson_string_capacity (void *unused)
356411
large_str[UINT32_MAX - 2u] = 's'; // Restore.
357412
}
358413

414+
// Can truncate strings of length close to UINT32_MAX - 1.
415+
{
416+
large_str[UINT32_MAX - 1u] = '\0'; // Set size.
417+
bson_string_t *str = bson_string_new (large_str);
418+
bson_string_truncate (str, UINT32_MAX - 2); // Truncate one character.
419+
ASSERT_CMPSIZE_T (strlen (str->str), ==, UINT32_MAX - 2u);
420+
bson_string_free (str, true);
421+
large_str[UINT32_MAX - 1u] = 's'; // Restore.
422+
}
423+
359424
bson_free (large_str);
360425
}
361426

@@ -385,4 +450,5 @@ test_string_install (TestSuite *suite)
385450
TestSuite_Add (suite, "/bson/string/strcasecmp", test_bson_strcasecmp);
386451
TestSuite_AddFull (
387452
suite, "/bson/string/capacity", test_bson_string_capacity, NULL, NULL, skip_if_no_large_allocations);
453+
TestSuite_Add (suite, "/bson/string/truncate", test_bson_string_truncate);
388454
}

0 commit comments

Comments
 (0)