Skip to content

Commit aee2821

Browse files
committed
CDRIVER-5734 fix max length string handling (#1798)
* relocate `skip_if_no_large_allocations` to share with new test * test calling `bson_value_copy` with UINT32_MAX length string (not NULL terminated) * do not try to NULL terminate if not possible * document length limitation
1 parent 3a73242 commit aee2821

File tree

6 files changed

+43
-13
lines changed

6 files changed

+43
-13
lines changed

src/libbson/doc/bson_value_copy.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,6 @@ Description
2222

2323
This function will copy the boxed content in ``src`` into ``dst``. ``dst`` must be freed with :symbol:`bson_value_destroy()` when no longer in use.
2424

25+
.. note::
26+
27+
If ``src`` represents a BSON UTF-8 string, :symbol:`bson_value_copy` attempts to NULL terminate the copied string in ``dst``. If ``src.value.v_utf8.len`` is `SIZE_MAX`, the copied string is (necessarily) not NULL terminated.

src/libbson/src/bson/bson-value.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,18 @@ bson_value_copy (const bson_value_t *src, /* IN */
3838
case BSON_TYPE_UTF8:
3939
BSON_ASSERT (mcommon_in_range_size_t_unsigned (src->value.v_utf8.len));
4040
size_t utf8_len_sz = (size_t) src->value.v_utf8.len;
41-
BSON_ASSERT (utf8_len_sz <= SIZE_MAX - 1);
42-
dst->value.v_utf8.len = src->value.v_utf8.len;
43-
dst->value.v_utf8.str = bson_malloc (utf8_len_sz + 1);
44-
memcpy (dst->value.v_utf8.str, src->value.v_utf8.str, dst->value.v_utf8.len);
45-
dst->value.v_utf8.str[dst->value.v_utf8.len] = '\0';
41+
if (utf8_len_sz == SIZE_MAX) {
42+
// If the string is at maximum length, do not NULL terminate. The source necessarily cannot fit it.
43+
dst->value.v_utf8.len = src->value.v_utf8.len;
44+
dst->value.v_utf8.str = bson_malloc (utf8_len_sz);
45+
memcpy (dst->value.v_utf8.str, src->value.v_utf8.str, dst->value.v_utf8.len);
46+
} else {
47+
// There is room in destination to NULL terminate.
48+
dst->value.v_utf8.len = src->value.v_utf8.len;
49+
dst->value.v_utf8.str = bson_malloc (utf8_len_sz + 1);
50+
memcpy (dst->value.v_utf8.str, src->value.v_utf8.str, dst->value.v_utf8.len);
51+
dst->value.v_utf8.str[dst->value.v_utf8.len] = '\0';
52+
}
4653
break;
4754
case BSON_TYPE_DOCUMENT:
4855
case BSON_TYPE_ARRAY:

src/libbson/tests/test-string.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -482,14 +482,6 @@ test_bson_string_alloc (void)
482482
bson_string_free (str, true);
483483
}
484484

485-
static int
486-
skip_if_no_large_allocations (void)
487-
{
488-
// Skip tests requiring large allocations.
489-
// Large allocations were observed to fail when run with TSan, and are time consuming with ASan.
490-
return test_framework_getenv_bool ("MONGOC_TEST_LARGE_ALLOCATIONS");
491-
}
492-
493485
void
494486
test_string_install (TestSuite *suite)
495487
{

src/libbson/tests/test-value.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include <bson/bson.h>
2020

2121
#include "TestSuite.h"
22+
#include "test-libmongoc.h" // skip_if_no_large_allocations
2223

2324

2425
static void
@@ -128,10 +129,25 @@ test_value_decimal128 (void)
128129
bson_destroy (&other);
129130
}
130131

132+
static void
133+
test_value_big_copy (void *unused)
134+
{
135+
BSON_UNUSED (unused);
136+
char *big_string = bson_malloc (UINT32_MAX);
137+
ASSERT (big_string);
138+
memset (big_string, UINT32_MAX, 'a'); // `big_string` is not NULL-terminated.
139+
140+
bson_value_t dst;
141+
bson_value_t src = {.value_type = BSON_TYPE_UTF8, .value = {.v_utf8 = {.str = big_string, .len = UINT32_MAX}}};
142+
bson_value_copy (&src, &dst);
143+
bson_free (big_string);
144+
bson_value_destroy (&dst);
145+
}
131146

132147
void
133148
test_value_install (TestSuite *suite)
134149
{
135150
TestSuite_Add (suite, "/bson/value/basic", test_value_basic);
136151
TestSuite_Add (suite, "/bson/value/decimal128", test_value_decimal128);
152+
TestSuite_AddFull (suite, "/bson/value/big_copy", test_value_big_copy, NULL, NULL, skip_if_no_large_allocations);
137153
}

src/libmongoc/tests/test-libmongoc.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2634,3 +2634,9 @@ test_framework_skip_if_no_server_ssl (void)
26342634
}
26352635
return 0; // Skip.
26362636
}
2637+
2638+
int
2639+
skip_if_no_large_allocations (void)
2640+
{
2641+
return test_framework_getenv_bool ("MONGOC_TEST_LARGE_ALLOCATIONS");
2642+
}

src/libmongoc/tests/test-libmongoc.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,4 +288,10 @@ test_framework_is_loadbalanced (void);
288288
int
289289
test_framework_skip_if_no_server_ssl (void);
290290

291+
292+
// `skip_if_no_large_allocations` skip tests requiring large allocations.
293+
// Large allocations were observed to fail when run with TSan, and are time consuming with ASan.
294+
int
295+
skip_if_no_large_allocations (void);
296+
291297
#endif

0 commit comments

Comments
 (0)