[CDRIVER-1921] mongoc_cursor_new_from_command_reply fails with static bson reply Created: 16/Nov/16  Updated: 27/Nov/16  Resolved: 17/Nov/16

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: None
Fix Version/s: 1.5.0

Type: Bug Priority: Major - P3
Reporter: David Golden Assignee: A. Jesse Jiryu Davis
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 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,



 Comments   
Comment by Githook User [ 27/Nov/16 ]

Author:

{u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}

Message: CDRIVER-1921 cursor from static reply
Branch: r1.5
https://github.com/mongodb/mongo-c-driver/commit/8f428e974c62ad35db90581fef68eecd9db98afb

Comment by Githook User [ 17/Nov/16 ]

Author:

{u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}

Message: CDRIVER-1921 cursor from static reply
Branch: r1.5
https://github.com/mongodb/mongo-c-driver/commit/751c853df9f796dd59f8adf62c05062d59441a13

Comment by Githook User [ 17/Nov/16 ]

Author:

{u'username': u'xdg', u'name': u'David Golden', u'email': u'xdg@xdg.me'}

Message: CDRIVER-1921 cursor from static reply
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/111ad1a8fb39fa996c707fbf7f93b30f4b8c988e

Comment by David Golden [ 17/Nov/16 ]

https://github.com/mongodb/mongo-c-driver/pull/405

Comment by A. Jesse Jiryu Davis [ 17/Nov/16 ]

LGTM, want to submit a PR? We can include in 1.5.0.

Comment by David Golden [ 16/Nov/16 ]

Code available as https://github.com/xdg/mongo-c-driver/commit/899baf78ec9b0e94389c14e46b0edc7b994bbf81 if desired.

Generated at Wed Feb 07 21:13:37 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.