CDRIVER-938 prohibit calling unsafe client setters with pooled clients (#1944)

* log and return on bad calls with a pooled client
** reject a pooled client in `mongoc_client_set_ssl_opts` and `mongoc_client_set_stream_initiator`
* fix test calling `mongoc_client_set_stream_initiator` on a pooled client
* add doc notices

Author: kevinAlbs

NEWS

@@ -44,6 +44,8 @@ Unreleased (2.0.0)
     * New behavior: `authMechanismProperties=A:B,C:D:E,F:G` is parsed as `{'A': 'B': 'C': 'D:E', 'F': 'G'}`.
 * Calling `mongoc_bulk_operation_execute` on the same `mongoc_bulk_operation_t` repeatedly is an error. Previously this was only discouraged in documentation.
 * Consistently apply `__cdecl` calling convention to function declarations in public API. Intended to support consumers building their code using a different [default calling convention](https://learn.microsoft.com/en-us/cpp/build/reference/gd-gr-gv-gz-calling-convention) with MSVC. The mongoc and bson libraries only support being built with the `__cdecl` default calling convention.
+* `mongoc_client_set_ssl_opts` now ignores a pooled `mongoc_client_t` and logs an error. Use `mongoc_client_pool_set_ssl_opts` to set TLS options on a `mongoc_client_pool_t` before popping any clients.
+* `mongoc_client_set_ssl_stream_initiator` now ignores a pooled `mongoc_client_t` and logs an error.
 
 ## Removals
 

src/libmongoc/doc/mongoc_client_set_ssl_opts.rst

@@ -32,6 +32,8 @@ It is a programming error to call this function on a client from a
 :symbol:`mongoc_client_pool_set_ssl_opts` on the pool before popping any
 clients.
 
+.. versionchanged:: 2.0.0 This function logs an error and immediately returns if ``client`` is from a :symbol:`mongoc_client_pool_t`. Previously this function unsafely applied the options to the pooled client.
+
 Parameters
 ----------
 

src/libmongoc/doc/mongoc_client_set_stream_initiator.rst

@@ -15,6 +15,10 @@ Synopsis
 
 The :symbol:`mongoc_client_set_stream_initiator()` function shall associate a given :symbol:`mongoc_client_t` with a new stream initiator. This will completely replace the default transport (buffered TCP, possibly with TLS). The ``initiator`` should fulfill the :symbol:`mongoc_stream_t` contract. ``user_data`` is passed through to the ``initiator`` callback and may be used for whatever run time customization is necessary.
 
+It is a programming error to call this function on a :symbol:`mongoc_client_t` from a :symbol:`mongoc_client_pool_t`.
+
+.. versionchanged:: 2.0.0 This function logs an error and immediately returns if ``client`` is from a :symbol:`mongoc_client_pool_t`. Previously this function unsafely applied the initiator to the pooled client.
+
 If ``user_data`` is passed, it is the application's responsibility to ensure ``user_data`` remains valid for the lifetime of the client.
 
 Parameters

src/libmongoc/src/mongoc/mongoc-client-pool.c

@@ -276,7 +276,7 @@ _initialize_new_client (mongoc_client_pool_t *pool, mongoc_client_t *client)
    BSON_ASSERT_PARAM (client);
 
    /* for tests */
-   mongoc_client_set_stream_initiator (
+   _mongoc_client_set_stream_initiator_single_or_pooled (
       client, pool->topology->scanner->initiator, pool->topology->scanner->initiator_context);
 
    pool->client_initialized = true;
@@ -286,7 +286,7 @@ _initialize_new_client (mongoc_client_pool_t *pool, mongoc_client_t *client)
 
 #ifdef MONGOC_ENABLE_SSL
    if (pool->ssl_opts_set) {
-      mongoc_client_set_ssl_opts (client, &pool->ssl_opts);
+      _mongoc_client_set_ssl_opts_for_single_or_pooled (client, &pool->ssl_opts);
    }
 #endif
 }

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

@@ -229,6 +229,14 @@ mongoc_client_uses_server_api (const mongoc_client_t *client);
 bool
 mongoc_client_uses_loadbalanced (const mongoc_client_t *client);
 
+void
+_mongoc_client_set_ssl_opts_for_single_or_pooled (mongoc_client_t *client, const mongoc_ssl_opt_t *opts);
+
+void
+_mongoc_client_set_stream_initiator_single_or_pooled (mongoc_client_t *client,
+                                                      mongoc_stream_initiator_t initiator,
+                                                      void *user_data);
+
 BSON_END_DECLS
 
 #endif /* MONGOC_CLIENT_PRIVATE_H */

src/libmongoc/src/mongoc/mongoc-client.c

@@ -985,7 +985,22 @@ void
 mongoc_client_set_ssl_opts (mongoc_client_t *client, const mongoc_ssl_opt_t *opts)
 {
    BSON_ASSERT_PARAM (client);
-   BSON_ASSERT (opts);
+   BSON_ASSERT_PARAM (opts);
+
+   if (!client->topology->single_threaded) {
+      MONGOC_ERROR (
+         "mongoc_client_set_ssl_opts cannot be called on a pooled client. Use mongoc_client_pool_set_ssl_opts.");
+      return;
+   }
+
+   _mongoc_client_set_ssl_opts_for_single_or_pooled (client, opts);
+}
+
+void
+_mongoc_client_set_ssl_opts_for_single_or_pooled (mongoc_client_t *client, const mongoc_ssl_opt_t *opts)
+{
+   BSON_ASSERT_PARAM (client);
+   BSON_ASSERT_PARAM (opts);
 
    _mongoc_ssl_opts_cleanup (&client->ssl_opts, false /* don't free internal opts */);
 
@@ -1003,7 +1018,7 @@ mongoc_client_set_ssl_opts (mongoc_client_t *client, const mongoc_ssl_opt_t *opt
 #endif
    }
 }
-#endif
+#endif // MONGOC_ENABLE_SSL
 
 
 mongoc_client_t *
@@ -1118,7 +1133,7 @@ _mongoc_client_new_from_topology (mongoc_topology_t *topology)
       _mongoc_ssl_opts_from_uri (&ssl_opt, &internal_tls_opts, client->uri);
       /* sets use_ssl = true */
       /* this call creates an ssl ctx only if single-threaded, otherwise client inherits from pool */
-      mongoc_client_set_ssl_opts (client, &ssl_opt);
+      _mongoc_client_set_ssl_opts_for_single_or_pooled (client, &ssl_opt);
       _mongoc_client_set_internal_tls_opts (client, &internal_tls_opts);
    }
 #endif
@@ -2226,6 +2241,21 @@ mongoc_client_set_stream_initiator (mongoc_client_t *client, mongoc_stream_initi
 {
    BSON_ASSERT_PARAM (client);
 
+   if (!client->topology->single_threaded) {
+      MONGOC_ERROR ("mongoc_client_set_stream_initiator cannot be called on a pooled client.");
+      return;
+   }
+
+   _mongoc_client_set_stream_initiator_single_or_pooled (client, initiator, user_data);
+}
+
+void
+_mongoc_client_set_stream_initiator_single_or_pooled (mongoc_client_t *client,
+                                                      mongoc_stream_initiator_t initiator,
+                                                      void *user_data)
+{
+   BSON_ASSERT_PARAM (client);
+
    if (!initiator) {
       initiator = mongoc_client_default_stream_initiator;
       user_data = client;

src/libmongoc/tests/test-mongoc-client-pool.c

@@ -546,7 +546,38 @@ test_mongoc_client_pool_change_openssl_ctx (void)
 
    mongoc_client_pool_destroy (pool);
 }
-#endif
+#endif // MONGOC_ENABLE_SSL_OPENSSL
+
+#if defined(MONGOC_ENABLE_SSL)
+static void
+test_mongoc_client_set_ssl_opts_on_pool (void)
+{
+   // Test calling `mongoc_client_set_ssl_opts` on a pooled client logs an error.
+   mongoc_client_pool_t *pool = test_framework_new_default_client_pool ();
+   mongoc_client_t *client_from_pool = mongoc_client_pool_pop (pool);
+   capture_logs (true);
+   mongoc_client_set_ssl_opts (client_from_pool, mongoc_ssl_opt_get_default ());
+   ASSERT_CAPTURED_LOG ("mongoc_client_set_ssl_opts", MONGOC_LOG_LEVEL_ERROR, "cannot be called on a pooled client");
+   capture_logs (false);
+   mongoc_client_pool_push (pool, client_from_pool);
+   mongoc_client_pool_destroy (pool);
+}
+#endif // MONGOC_ENABLE_SSL
+
+static void
+test_mongoc_client_set_stream_initiator (void)
+{
+   // Test calling `mongoc_client_set_initiator` on a pooled client logs an error.
+   mongoc_client_pool_t *pool = test_framework_new_default_client_pool ();
+   mongoc_client_t *client_from_pool = mongoc_client_pool_pop (pool);
+   capture_logs (true);
+   mongoc_client_set_stream_initiator (client_from_pool, mongoc_client_default_stream_initiator, NULL);
+   ASSERT_CAPTURED_LOG (
+      "mongoc_client_set_stream_initiator", MONGOC_LOG_LEVEL_ERROR, "cannot be called on a pooled client");
+   capture_logs (false);
+   mongoc_client_pool_push (pool, client_from_pool);
+   mongoc_client_pool_destroy (pool);
+}
 
 void
 test_client_pool_install (TestSuite *suite)
@@ -593,4 +624,9 @@ test_client_pool_install (TestSuite *suite)
 #if defined(MONGOC_ENABLE_SSL_OPENSSL)
    TestSuite_Add (suite, "/ClientPool/openssl/change_ssl_opts", test_mongoc_client_pool_change_openssl_ctx);
 #endif
+
+#if defined(MONGOC_ENABLE_SSL)
+   TestSuite_AddLive (suite, "/ClientPool/mongoc_client_set_ssl_opts", test_mongoc_client_set_ssl_opts_on_pool);
+#endif // MONGOC_ENABLE_SSL
+   TestSuite_AddLive (suite, "/ClientPool/mongoc_client_set_stream_initiator", test_mongoc_client_set_stream_initiator);
 }

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

@@ -1322,7 +1322,7 @@ test_hello_on_unknown (void)
 
    client = mongoc_client_pool_pop (pool);
 
-   mongoc_client_set_stream_initiator (client, _initiator_fn, pool);
+   _mongoc_client_set_stream_initiator_single_or_pooled (client, _initiator_fn, pool);
 
    /* The other client marked the server as unknown after this client selected
     * the server and created a stream, but *before* constructing the initial