Skip to content

Commit ac38749

Browse files
authored
CDRIVER-6112 fix ownership transfer of mongoc_write_command_t (#2132)
* 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 e5c3235 commit ac38749

File tree

4 files changed

+63
-5
lines changed

4 files changed

+63
-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
@@ -62,7 +62,7 @@ typedef struct {
6262
uint32_t n_documents;
6363
mongoc_bulk_write_flags_t flags;
6464
int64_t operation_id;
65-
bson_t cmd_opts;
65+
bson_t *cmd_opts;
6666
} mongoc_write_command_t;
6767

6868

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,9 @@ _mongoc_write_command_init_bulk(
144144
command->flags = flags;
145145
command->operation_id = operation_id;
146146
if (!bson_empty0(opts)) {
147-
bson_copy_to(opts, &command->cmd_opts);
147+
command->cmd_opts = bson_copy(opts);
148148
} else {
149-
bson_init(&command->cmd_opts);
149+
command->cmd_opts = bson_new();
150150
}
151151

152152
_mongoc_buffer_init(&command->payload, NULL, 0, NULL, NULL);
@@ -668,7 +668,7 @@ _mongoc_write_opmsg(mongoc_write_command_t *command,
668668
? MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_NO
669669
: MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_YES;
670670

671-
BSON_ASSERT(bson_iter_init(&iter, &command->cmd_opts));
671+
BSON_ASSERT(bson_iter_init(&iter, command->cmd_opts));
672672
if (!mongoc_cmd_parts_append_opts(&parts, &iter, error)) {
673673
bson_destroy(&cmd);
674674
mongoc_cmd_parts_cleanup(&parts);
@@ -937,7 +937,7 @@ _mongoc_write_command_destroy(mongoc_write_command_t *command)
937937
ENTRY;
938938

939939
if (command) {
940-
bson_destroy(&command->cmd_opts);
940+
bson_destroy(command->cmd_opts);
941941
_mongoc_buffer_destroy(&command->payload);
942942
}
943943

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4800,6 +4800,54 @@ test_multiple_execution(void)
48004800
mongoc_client_destroy(client);
48014801
}
48024802

4803+
// `test_bulk_big_let` tests a bulk operation with a large let document to reproduce CDRIVER-6112:
4804+
static void
4805+
test_bulk_big_let(void *unused)
4806+
{
4807+
BSON_UNUSED(unused);
4808+
4809+
mongoc_client_t *client = test_framework_new_default_client();
4810+
mongoc_collection_t *coll = get_test_collection(client, "test_big_let");
4811+
bson_error_t error;
4812+
4813+
// Create bulk operation similar to PHP driver:
4814+
mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new(true /* ordered */);
4815+
4816+
// Set a large `let`: { "testDocument": { "a": "aaa..." } }
4817+
{
4818+
bson_t let = BSON_INITIALIZER, testDocument;
4819+
bson_append_document_begin(&let, "testDocument", -1, &testDocument);
4820+
4821+
// Append big string:
4822+
{
4823+
size_t num_chars = 79;
4824+
char *big_string = bson_malloc0(num_chars + 1);
4825+
memset(big_string, 'a', num_chars);
4826+
BSON_APPEND_UTF8(&testDocument, "a", big_string);
4827+
bson_free(big_string);
4828+
}
4829+
4830+
bson_append_document_end(&let, &testDocument);
4831+
mongoc_bulk_operation_set_let(bulk, &let);
4832+
bson_destroy(&let);
4833+
}
4834+
4835+
4836+
mongoc_bulk_operation_set_client(bulk, client);
4837+
mongoc_bulk_operation_set_database(bulk, "db");
4838+
mongoc_bulk_operation_set_collection(bulk, "coll");
4839+
4840+
mongoc_bulk_operation_update(
4841+
bulk, tmp_bson("{'_id': 1}"), tmp_bson("{'$set': {'document': '$$testDocument'}}"), true);
4842+
4843+
4844+
ASSERT_OR_PRINT(mongoc_bulk_operation_execute(bulk, NULL, &error), error);
4845+
4846+
mongoc_bulk_operation_destroy(bulk);
4847+
mongoc_collection_destroy(coll);
4848+
mongoc_client_destroy(client);
4849+
}
4850+
48034851

48044852
void
48054853
test_bulk_install(TestSuite *suite)
@@ -4978,4 +5026,11 @@ test_bulk_install(TestSuite *suite)
49785026
"/BulkOperation/set_client_updates_operation_id_when_client_changes",
49795027
test_bulk_write_set_client_updates_operation_id_when_client_changes);
49805028
TestSuite_AddLive(suite, "/BulkOperation/multiple_execution", test_multiple_execution);
5029+
TestSuite_AddFull(
5030+
suite,
5031+
"/BulkOperation/big_let",
5032+
test_bulk_big_let,
5033+
NULL,
5034+
NULL,
5035+
test_framework_skip_if_max_wire_version_less_than_13 /* 5.0+ for 'let' support in CRUD commands */);
49815036
}

0 commit comments

Comments
 (0)