Description
mongoc_cursor_new_from_command_reply() internally eventually calls bson_steal() on the provided reply bson_t pointer and does not check it's return value. bson_steal fails for read-only bson_t structures, so mongoc_cursor_new_from_command_reply fails to take ownership of the provided bson.
Sample failing test is here:
diff --git a/tests/test-mongoc-cursor.c b/tests/test-mongoc-cursor.c
|
index 9af4348..d2198c7 100644
|
--- a/tests/test-mongoc-cursor.c
|
+++ b/tests/test-mongoc-cursor.c
|
@@ -734,6 +734,36 @@ test_cursor_new_invalid (void)
|
mongoc_client_destroy (client);
|
}
|
|
+static void
|
+test_cursor_new_static (void)
|
+{
|
+ mongoc_client_t *client;
|
+ bson_error_t error;
|
+ mongoc_cursor_t *cursor;
|
+ client = test_framework_client_new ();
|
+
|
+ char *j = "{ \"ok\":1, \"cursor\":{ \"id\":0, \"ns\":\"test.foo\", \"firstBatch\":[ { \"x\":1 }, {\"x\":2 } ] } }";
|
+ bson_t* b = bson_new_from_json((uint8_t*) j, strlen(j), &error);
|
+ ASSERT_OR_PRINT (b, error);
|
+
|
+ bson_t bs;
|
+ ASSERT (bson_init_static(&bs, bson_get_data(b), b->len));
|
+
|
+ /* verify that original works */
|
+ cursor = mongoc_cursor_new_from_command_reply (client, bson_copy(b), 0);
|
+ ASSERT (cursor);
|
+ ASSERT (!mongoc_cursor_error(cursor, &error));
|
+
|
+ /* then test static bson */
|
+ cursor = mongoc_cursor_new_from_command_reply (client, &bs, 0);
|
+ ASSERT (cursor);
|
+ ASSERT (!mongoc_cursor_error(cursor, &error));
|
+
|
+ bson_destroy(b);
|
+ mongoc_cursor_destroy (cursor);
|
+ mongoc_client_destroy (client);
|
+}
|
+
|
|
static void
|
test_cursor_hint_errors (void)
|
@@ -1516,6 +1546,7 @@ test_cursor_install (TestSuite *suite)
|
test_cursor_new_from_find_batches, NULL, NULL,
|
test_framework_skip_if_max_wire_version_less_than_4);
|
TestSuite_AddLive (suite, "/Cursor/new_invalid", test_cursor_new_invalid);
|
+ TestSuite_Add (suite, "/Cursor/new_static", test_cursor_new_static);
|
TestSuite_AddLive (suite, "/Cursor/hint/errors", test_cursor_hint_errors);
|
TestSuite_Add (suite, "/Cursor/hint/single/secondary", test_hint_single_secondary);
|
TestSuite_Add (suite, "/Cursor/hint/single/primary", test_hint_single_primary); |
Some possible options for fixes:
- copy the bson instead of stealing if static
- document that mongoc_cursor_command_new_from_command_reply requires a non-static reply argument (and suggest the use of bson_copy for that case)
For example, a trivial fix (though one that still doesn't check the final bson_steal return value):
diff --git a/src/mongoc/mongoc-cursor-cursorid.c b/src/mongoc/mongoc-cursor-cursorid.c
|
index 2e11d3b..fbd62c5 100644
|
--- a/src/mongoc/mongoc-cursor-cursorid.c
|
+++ b/src/mongoc/mongoc-cursor-cursorid.c
|
@@ -373,7 +373,7 @@ _mongoc_cursor_cursorid_init_with_reply (mongoc_cursor_t *cursor,
|
BSON_ASSERT (cid);
|
|
|
bson_destroy (&cid->array);
|
- bson_steal (&cid->array, reply);
|
+ bson_steal (&cid->array, reply) || bson_steal(&cid->array, bson_copy(reply));
|
|
|
if (!_mongoc_cursor_cursorid_start_batch (cursor)) {
|
bson_set_error (&cursor->error, |