|
| 1 | +From 27419bebfa8c0772e220592c86cf700b1ce2995d Mon Sep 17 00:00:00 2001 |
| 2 | +From: Kevin Albertson <kevin.albertson@mongodb.com> |
| 3 | +Date: Mon, 6 Oct 2025 11:38:22 -0400 |
| 4 | +Subject: [PATCH] CDRIVER-6112 fix ownership transfer of |
| 5 | + `mongoc_write_command_t` (#2132) (#2137) |
| 6 | + |
| 7 | +* add regression test |
| 8 | +* do not memcpy `bson_t` struct in array |
| 9 | + * `memcpy` does not correctly transfer ownership of `bson_t`. Instead: heap allocate `bson_t`. |
| 10 | +* warn against using `bson_t` in `mongoc_array_t` |
| 11 | +--- |
| 12 | + .../src/mongoc/mongoc-array-private.h | 3 + |
| 13 | + .../src/mongoc/mongoc-write-command-private.h | 2 +- |
| 14 | + .../src/mongoc/mongoc-write-command.c | 8 +-- |
| 15 | + src/libmongoc/tests/test-mongoc-bulk.c | 56 +++++++++++++++++++ |
| 16 | + 4 files changed, 64 insertions(+), 5 deletions(-) |
| 17 | + |
| 18 | +diff --git a/src/libmongoc/src/mongoc/mongoc-array-private.h b/src/libmongoc/src/mongoc/mongoc-array-private.h |
| 19 | +index 9956224b34..c8de6f1f52 100644 |
| 20 | +--- a/src/libmongoc/src/mongoc/mongoc-array-private.h |
| 21 | ++++ b/src/libmongoc/src/mongoc/mongoc-array-private.h |
| 22 | +@@ -25,6 +25,9 @@ |
| 23 | + BSON_BEGIN_DECLS |
| 24 | + |
| 25 | + |
| 26 | ++// mongoc_array_t stores an array of objects of type T. |
| 27 | ++// |
| 28 | ++// T must be trivially relocatable. In particular, `bson_t` is not trivially relocatable (CDRIVER-6113). |
| 29 | + typedef struct _mongoc_array_t mongoc_array_t; |
| 30 | + |
| 31 | + |
| 32 | +diff --git a/src/libmongoc/src/mongoc/mongoc-write-command-private.h b/src/libmongoc/src/mongoc/mongoc-write-command-private.h |
| 33 | +index 85121594e0..c1bf751e01 100644 |
| 34 | +--- a/src/libmongoc/src/mongoc/mongoc-write-command-private.h |
| 35 | ++++ b/src/libmongoc/src/mongoc/mongoc-write-command-private.h |
| 36 | +@@ -61,7 +61,7 @@ typedef struct { |
| 37 | + uint32_t n_documents; |
| 38 | + mongoc_bulk_write_flags_t flags; |
| 39 | + int64_t operation_id; |
| 40 | +- bson_t cmd_opts; |
| 41 | ++ bson_t *cmd_opts; |
| 42 | + } mongoc_write_command_t; |
| 43 | + |
| 44 | + |
| 45 | +diff --git a/src/libmongoc/src/mongoc/mongoc-write-command.c b/src/libmongoc/src/mongoc/mongoc-write-command.c |
| 46 | +index a375d8f200..36f2470acb 100644 |
| 47 | +--- a/src/libmongoc/src/mongoc/mongoc-write-command.c |
| 48 | ++++ b/src/libmongoc/src/mongoc/mongoc-write-command.c |
| 49 | +@@ -143,9 +143,9 @@ _mongoc_write_command_init_bulk ( |
| 50 | + command->flags = flags; |
| 51 | + command->operation_id = operation_id; |
| 52 | + if (!bson_empty0 (opts)) { |
| 53 | +- bson_copy_to (opts, &command->cmd_opts); |
| 54 | ++ command->cmd_opts = bson_copy (opts); |
| 55 | + } else { |
| 56 | +- bson_init (&command->cmd_opts); |
| 57 | ++ command->cmd_opts = bson_new (); |
| 58 | + } |
| 59 | + |
| 60 | + _mongoc_buffer_init (&command->payload, NULL, 0, NULL, NULL); |
| 61 | +@@ -671,7 +671,7 @@ _mongoc_write_opmsg (mongoc_write_command_t *command, |
| 62 | + ? MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_NO |
| 63 | + : MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_YES; |
| 64 | + |
| 65 | +- BSON_ASSERT (bson_iter_init (&iter, &command->cmd_opts)); |
| 66 | ++ BSON_ASSERT (bson_iter_init (&iter, command->cmd_opts)); |
| 67 | + if (!mongoc_cmd_parts_append_opts (&parts, &iter, error)) { |
| 68 | + bson_destroy (&cmd); |
| 69 | + mongoc_cmd_parts_cleanup (&parts); |
| 70 | +@@ -944,7 +944,7 @@ _mongoc_write_command_destroy (mongoc_write_command_t *command) |
| 71 | + ENTRY; |
| 72 | + |
| 73 | + if (command) { |
| 74 | +- bson_destroy (&command->cmd_opts); |
| 75 | ++ bson_destroy (command->cmd_opts); |
| 76 | + _mongoc_buffer_destroy (&command->payload); |
| 77 | + } |
| 78 | + |
| 79 | +diff --git a/src/libmongoc/tests/test-mongoc-bulk.c b/src/libmongoc/tests/test-mongoc-bulk.c |
| 80 | +index 357893ce1c..e4666c1db3 100644 |
| 81 | +--- a/src/libmongoc/tests/test-mongoc-bulk.c |
| 82 | ++++ b/src/libmongoc/tests/test-mongoc-bulk.c |
| 83 | +@@ -4768,6 +4768,55 @@ test_bulk_write_set_client_updates_operation_id_when_client_changes (void) |
| 84 | + mock_server_destroy (mock_server); |
| 85 | + } |
| 86 | + |
| 87 | ++// `test_bulk_big_let` tests a bulk operation with a large let document to reproduce CDRIVER-6112: |
| 88 | ++static void |
| 89 | ++test_bulk_big_let (void *unused) |
| 90 | ++{ |
| 91 | ++ BSON_UNUSED (unused); |
| 92 | ++ |
| 93 | ++ mongoc_client_t *client = test_framework_new_default_client (); |
| 94 | ++ mongoc_collection_t *coll = get_test_collection (client, "test_big_let"); |
| 95 | ++ bson_error_t error; |
| 96 | ++ |
| 97 | ++ // Create bulk operation similar to PHP driver: |
| 98 | ++ mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true /* ordered */); |
| 99 | ++ |
| 100 | ++ // Set a large `let`: { "testDocument": { "a": "aaa..." } } |
| 101 | ++ { |
| 102 | ++ bson_t let = BSON_INITIALIZER, testDocument; |
| 103 | ++ bson_append_document_begin (&let, "testDocument", -1, &testDocument); |
| 104 | ++ |
| 105 | ++ // Append big string: |
| 106 | ++ { |
| 107 | ++ size_t num_chars = 79; |
| 108 | ++ char *big_string = bson_malloc0 (num_chars + 1); |
| 109 | ++ memset (big_string, 'a', num_chars); |
| 110 | ++ BSON_APPEND_UTF8 (&testDocument, "a", big_string); |
| 111 | ++ bson_free (big_string); |
| 112 | ++ } |
| 113 | ++ |
| 114 | ++ bson_append_document_end (&let, &testDocument); |
| 115 | ++ mongoc_bulk_operation_set_let (bulk, &let); |
| 116 | ++ bson_destroy (&let); |
| 117 | ++ } |
| 118 | ++ |
| 119 | ++ |
| 120 | ++ mongoc_bulk_operation_set_client (bulk, client); |
| 121 | ++ mongoc_bulk_operation_set_database (bulk, "db"); |
| 122 | ++ mongoc_bulk_operation_set_collection (bulk, "coll"); |
| 123 | ++ |
| 124 | ++ mongoc_bulk_operation_update ( |
| 125 | ++ bulk, tmp_bson ("{'_id': 1}"), tmp_bson ("{'$set': {'document': '$$testDocument'}}"), true); |
| 126 | ++ |
| 127 | ++ |
| 128 | ++ ASSERT_OR_PRINT (mongoc_bulk_operation_execute (bulk, NULL, &error), error); |
| 129 | ++ |
| 130 | ++ mongoc_bulk_operation_destroy (bulk); |
| 131 | ++ mongoc_collection_destroy (coll); |
| 132 | ++ mongoc_client_destroy (client); |
| 133 | ++} |
| 134 | ++ |
| 135 | ++ |
| 136 | + void |
| 137 | + test_bulk_install (TestSuite *suite) |
| 138 | + { |
| 139 | +@@ -4946,4 +4995,11 @@ test_bulk_install (TestSuite *suite) |
| 140 | + TestSuite_AddMockServerTest (suite, |
| 141 | + "/BulkOperation/set_client_updates_operation_id_when_client_changes", |
| 142 | + test_bulk_write_set_client_updates_operation_id_when_client_changes); |
| 143 | ++ TestSuite_AddFull ( |
| 144 | ++ suite, |
| 145 | ++ "/BulkOperation/big_let", |
| 146 | ++ test_bulk_big_let, |
| 147 | ++ NULL, |
| 148 | ++ NULL, |
| 149 | ++ test_framework_skip_if_max_wire_version_less_than_13 /* 5.0+ for 'let' support in CRUD commands */); |
| 150 | + } |
| 151 | +-- |
| 152 | +2.39.5 |
| 153 | + |
0 commit comments