Skip to content

Commit 27419be

Browse files
authored
CDRIVER-6112 fix ownership transfer of mongoc_write_command_t (#2132) (#2137)
* add regression test * do not memcpy `bson_t` struct in array * `memcpy` does not correctly transfer ownership of `bson_t`. Instead: heap allocate `bson_t`. * warn against using `bson_t` in `mongoc_array_t`
1 parent c07d664 commit 27419be

File tree

4 files changed

+64
-5
lines changed

4 files changed

+64
-5
lines changed

src/libmongoc/src/mongoc/mongoc-array-private.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
BSON_BEGIN_DECLS
2626

2727

28+
// mongoc_array_t stores an array of objects of type T.
29+
//
30+
// T must be trivially relocatable. In particular, `bson_t` is not trivially relocatable (CDRIVER-6113).
2831
typedef struct _mongoc_array_t mongoc_array_t;
2932

3033

src/libmongoc/src/mongoc/mongoc-write-command-private.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ typedef struct {
6161
uint32_t n_documents;
6262
mongoc_bulk_write_flags_t flags;
6363
int64_t operation_id;
64-
bson_t cmd_opts;
64+
bson_t *cmd_opts;
6565
} mongoc_write_command_t;
6666

6767

src/libmongoc/src/mongoc/mongoc-write-command.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ _mongoc_write_command_init_bulk (
143143
command->flags = flags;
144144
command->operation_id = operation_id;
145145
if (!bson_empty0 (opts)) {
146-
bson_copy_to (opts, &command->cmd_opts);
146+
command->cmd_opts = bson_copy (opts);
147147
} else {
148-
bson_init (&command->cmd_opts);
148+
command->cmd_opts = bson_new ();
149149
}
150150

151151
_mongoc_buffer_init (&command->payload, NULL, 0, NULL, NULL);
@@ -671,7 +671,7 @@ _mongoc_write_opmsg (mongoc_write_command_t *command,
671671
? MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_NO
672672
: MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_YES;
673673

674-
BSON_ASSERT (bson_iter_init (&iter, &command->cmd_opts));
674+
BSON_ASSERT (bson_iter_init (&iter, command->cmd_opts));
675675
if (!mongoc_cmd_parts_append_opts (&parts, &iter, error)) {
676676
bson_destroy (&cmd);
677677
mongoc_cmd_parts_cleanup (&parts);
@@ -944,7 +944,7 @@ _mongoc_write_command_destroy (mongoc_write_command_t *command)
944944
ENTRY;
945945

946946
if (command) {
947-
bson_destroy (&command->cmd_opts);
947+
bson_destroy (command->cmd_opts);
948948
_mongoc_buffer_destroy (&command->payload);
949949
}
950950

src/libmongoc/tests/test-mongoc-bulk.c

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4768,6 +4768,55 @@ test_bulk_write_set_client_updates_operation_id_when_client_changes (void)
47684768
mock_server_destroy (mock_server);
47694769
}
47704770

4771+
// `test_bulk_big_let` tests a bulk operation with a large let document to reproduce CDRIVER-6112:
4772+
static void
4773+
test_bulk_big_let (void *unused)
4774+
{
4775+
BSON_UNUSED (unused);
4776+
4777+
mongoc_client_t *client = test_framework_new_default_client ();
4778+
mongoc_collection_t *coll = get_test_collection (client, "test_big_let");
4779+
bson_error_t error;
4780+
4781+
// Create bulk operation similar to PHP driver:
4782+
mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true /* ordered */);
4783+
4784+
// Set a large `let`: { "testDocument": { "a": "aaa..." } }
4785+
{
4786+
bson_t let = BSON_INITIALIZER, testDocument;
4787+
bson_append_document_begin (&let, "testDocument", -1, &testDocument);
4788+
4789+
// Append big string:
4790+
{
4791+
size_t num_chars = 79;
4792+
char *big_string = bson_malloc0 (num_chars + 1);
4793+
memset (big_string, 'a', num_chars);
4794+
BSON_APPEND_UTF8 (&testDocument, "a", big_string);
4795+
bson_free (big_string);
4796+
}
4797+
4798+
bson_append_document_end (&let, &testDocument);
4799+
mongoc_bulk_operation_set_let (bulk, &let);
4800+
bson_destroy (&let);
4801+
}
4802+
4803+
4804+
mongoc_bulk_operation_set_client (bulk, client);
4805+
mongoc_bulk_operation_set_database (bulk, "db");
4806+
mongoc_bulk_operation_set_collection (bulk, "coll");
4807+
4808+
mongoc_bulk_operation_update (
4809+
bulk, tmp_bson ("{'_id': 1}"), tmp_bson ("{'$set': {'document': '$$testDocument'}}"), true);
4810+
4811+
4812+
ASSERT_OR_PRINT (mongoc_bulk_operation_execute (bulk, NULL, &error), error);
4813+
4814+
mongoc_bulk_operation_destroy (bulk);
4815+
mongoc_collection_destroy (coll);
4816+
mongoc_client_destroy (client);
4817+
}
4818+
4819+
47714820
void
47724821
test_bulk_install (TestSuite *suite)
47734822
{
@@ -4946,4 +4995,11 @@ test_bulk_install (TestSuite *suite)
49464995
TestSuite_AddMockServerTest (suite,
49474996
"/BulkOperation/set_client_updates_operation_id_when_client_changes",
49484997
test_bulk_write_set_client_updates_operation_id_when_client_changes);
4998+
TestSuite_AddFull (
4999+
suite,
5000+
"/BulkOperation/big_let",
5001+
test_bulk_big_let,
5002+
NULL,
5003+
NULL,
5004+
test_framework_skip_if_max_wire_version_less_than_13 /* 5.0+ for 'let' support in CRUD commands */);
49495005
}

0 commit comments

Comments
 (0)