PYTHON-5306 - Fix use of public MongoClient attributes before connection

Author: NoahStapp

pymongo/asynchronous/mongo_client.py

@@ -109,6 +109,7 @@
 )
 from pymongo.read_preferences import ReadPreference, _ServerMode
 from pymongo.results import ClientBulkWriteResult
+from pymongo.server_description import ServerDescription
 from pymongo.server_selectors import writable_server_selector
 from pymongo.server_type import SERVER_TYPE
 from pymongo.topology_description import TOPOLOGY_TYPE, TopologyDescription
@@ -779,7 +780,7 @@ def __init__(
         keyword_opts["document_class"] = doc_class
         self._resolve_srv_info: dict[str, Any] = {"keyword_opts": keyword_opts}
 
-        seeds = set()
+        self._seeds = set()
         is_srv = False
         username = None
         password = None
@@ -804,18 +805,18 @@ def __init__(
                     srv_max_hosts=srv_max_hosts,
                 )
                 is_srv = entity.startswith(SRV_SCHEME)
-                seeds.update(res["nodelist"])
+                self._seeds.update(res["nodelist"])
                 username = res["username"] or username
                 password = res["password"] or password
                 dbase = res["database"] or dbase
                 opts = res["options"]
                 fqdn = res["fqdn"]
             else:
-                seeds.update(split_hosts(entity, self._port))
-        if not seeds:
+                self._seeds.update(split_hosts(entity, self._port))
+        if not self._seeds:
             raise ConfigurationError("need to specify at least one host")
 
-        for hostname in [node[0] for node in seeds]:
+        for hostname in [node[0] for node in self._seeds]:
             if _detect_external_db(hostname):
                 break
 
@@ -838,7 +839,7 @@ def __init__(
             srv_service_name = opts.get("srvServiceName", common.SRV_SERVICE_NAME)
 
         srv_max_hosts = srv_max_hosts or opts.get("srvmaxhosts")
-        opts = self._normalize_and_validate_options(opts, seeds)
+        opts = self._normalize_and_validate_options(opts, self._seeds)
 
         # Username and password passed as kwargs override user info in URI.
         username = opts.get("username", username)
@@ -857,7 +858,7 @@ def __init__(
                 "username": username,
                 "password": password,
                 "dbase": dbase,
-                "seeds": seeds,
+                "seeds": self._seeds,
                 "fqdn": fqdn,
                 "srv_service_name": srv_service_name,
                 "pool_class": pool_class,
@@ -874,7 +875,7 @@ def __init__(
         )
 
         if not is_srv:
-            self._init_based_on_options(seeds, srv_max_hosts, srv_service_name)
+            self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name)
 
         self._opened = False
         self._closed = False
@@ -1205,6 +1206,18 @@ def topology_description(self) -> TopologyDescription:
 
         .. versionadded:: 4.0
         """
+        if self._topology is None:
+            servers = {
+                (host, self._port): ServerDescription((host, self._port)) for host in self._seeds
+            }
+            return TopologyDescription(
+                TOPOLOGY_TYPE.Unknown,
+                servers,
+                None,
+                None,
+                None,
+                TopologySettings(),
+            )
         return self._topology.description
 
     @property
@@ -1218,6 +1231,8 @@ def nodes(self) -> FrozenSet[_Address]:
           to any servers, or a network partition causes it to lose connection
           to all servers.
         """
+        if self._topology is None:
+            return frozenset()
         description = self._topology.description
         return frozenset(s.address for s in description.known_servers)
 
@@ -1576,6 +1591,8 @@ async def address(self) -> Optional[tuple[str, int]]:
 
         .. versionadded:: 3.0
         """
+        if self._topology is None:
+            await self._get_topology()
         topology_type = self._topology._description.topology_type
         if (
             topology_type == TOPOLOGY_TYPE.Sharded
@@ -1598,6 +1615,8 @@ async def primary(self) -> Optional[tuple[str, int]]:
         .. versionadded:: 3.0
            AsyncMongoClient gained this property in version 3.0.
         """
+        if self._topology is None:
+            await self._get_topology()
         return await self._topology.get_primary()  # type: ignore[return-value]
 
     @property
@@ -1611,6 +1630,8 @@ async def secondaries(self) -> set[_Address]:
         .. versionadded:: 3.0
            AsyncMongoClient gained this property in version 3.0.
         """
+        if self._topology is None:
+            await self._get_topology()
         return await self._topology.get_secondaries()
 
     @property
@@ -1621,6 +1642,8 @@ async def arbiters(self) -> set[_Address]:
         connected to a replica set, there are no arbiters, or this client was
         created without the `replicaSet` option.
         """
+        if self._topology is None:
+            await self._get_topology()
         return await self._topology.get_arbiters()
 
     @property

pymongo/synchronous/mongo_client.py

@@ -101,6 +101,7 @@
 )
 from pymongo.read_preferences import ReadPreference, _ServerMode
 from pymongo.results import ClientBulkWriteResult
+from pymongo.server_description import ServerDescription
 from pymongo.server_selectors import writable_server_selector
 from pymongo.server_type import SERVER_TYPE
 from pymongo.synchronous import client_session, database, uri_parser
@@ -777,7 +778,7 @@ def __init__(
         keyword_opts["document_class"] = doc_class
         self._resolve_srv_info: dict[str, Any] = {"keyword_opts": keyword_opts}
 
-        seeds = set()
+        self._seeds = set()
         is_srv = False
         username = None
         password = None
@@ -802,18 +803,18 @@ def __init__(
                     srv_max_hosts=srv_max_hosts,
                 )
                 is_srv = entity.startswith(SRV_SCHEME)
-                seeds.update(res["nodelist"])
+                self._seeds.update(res["nodelist"])
                 username = res["username"] or username
                 password = res["password"] or password
                 dbase = res["database"] or dbase
                 opts = res["options"]
                 fqdn = res["fqdn"]
             else:
-                seeds.update(split_hosts(entity, self._port))
-        if not seeds:
+                self._seeds.update(split_hosts(entity, self._port))
+        if not self._seeds:
             raise ConfigurationError("need to specify at least one host")
 
-        for hostname in [node[0] for node in seeds]:
+        for hostname in [node[0] for node in self._seeds]:
             if _detect_external_db(hostname):
                 break
 
@@ -836,7 +837,7 @@ def __init__(
             srv_service_name = opts.get("srvServiceName", common.SRV_SERVICE_NAME)
 
         srv_max_hosts = srv_max_hosts or opts.get("srvmaxhosts")
-        opts = self._normalize_and_validate_options(opts, seeds)
+        opts = self._normalize_and_validate_options(opts, self._seeds)
 
         # Username and password passed as kwargs override user info in URI.
         username = opts.get("username", username)
@@ -855,7 +856,7 @@ def __init__(
                 "username": username,
                 "password": password,
                 "dbase": dbase,
-                "seeds": seeds,
+                "seeds": self._seeds,
                 "fqdn": fqdn,
                 "srv_service_name": srv_service_name,
                 "pool_class": pool_class,
@@ -872,7 +873,7 @@ def __init__(
         )
 
         if not is_srv:
-            self._init_based_on_options(seeds, srv_max_hosts, srv_service_name)
+            self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name)
 
         self._opened = False
         self._closed = False
@@ -1203,6 +1204,18 @@ def topology_description(self) -> TopologyDescription:
 
         .. versionadded:: 4.0
         """
+        if self._topology is None:
+            servers = {
+                (host, self._port): ServerDescription((host, self._port)) for host in self._seeds
+            }
+            return TopologyDescription(
+                TOPOLOGY_TYPE.Unknown,
+                servers,
+                None,
+                None,
+                None,
+                TopologySettings(),
+            )
         return self._topology.description
 
     @property
@@ -1216,6 +1229,8 @@ def nodes(self) -> FrozenSet[_Address]:
           to any servers, or a network partition causes it to lose connection
           to all servers.
         """
+        if self._topology is None:
+            return frozenset()
         description = self._topology.description
         return frozenset(s.address for s in description.known_servers)
 
@@ -1570,6 +1585,8 @@ def address(self) -> Optional[tuple[str, int]]:
 
         .. versionadded:: 3.0
         """
+        if self._topology is None:
+            self._get_topology()
         topology_type = self._topology._description.topology_type
         if (
             topology_type == TOPOLOGY_TYPE.Sharded
@@ -1592,6 +1609,8 @@ def primary(self) -> Optional[tuple[str, int]]:
         .. versionadded:: 3.0
            MongoClient gained this property in version 3.0.
         """
+        if self._topology is None:
+            self._get_topology()
         return self._topology.get_primary()  # type: ignore[return-value]
 
     @property
@@ -1605,6 +1624,8 @@ def secondaries(self) -> set[_Address]:
         .. versionadded:: 3.0
            MongoClient gained this property in version 3.0.
         """
+        if self._topology is None:
+            self._get_topology()
         return self._topology.get_secondaries()
 
     @property
@@ -1615,6 +1636,8 @@ def arbiters(self) -> set[_Address]:
         connected to a replica set, there are no arbiters, or this client was
         created without the `replicaSet` option.
         """
+        if self._topology is None:
+            self._get_topology()
         return self._topology.get_arbiters()
 
     @property

test/asynchronous/test_client.py

@@ -816,6 +816,30 @@ async def test_constants(self):
     async def test_init_disconnected(self):
         host, port = await async_client_context.host, await async_client_context.port
         c = await self.async_rs_or_single_client(connect=False)
+        # nodes returns an empty set if not connected
+        self.assertEqual(c.nodes, frozenset())
+        # topology_description returns the initial seed description if not connected
+        topology_description = c.topology_description
+        self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown)
+        self.assertEqual(
+            topology_description.server_descriptions(),
+            {(host, port): ServerDescription((host, port))},
+        )
+        # address causes client to block until connected
+        self.assertIsNotNone(await c.address)
+        c = await self.async_rs_or_single_client(connect=False)
+        # primary causes client to block until connected
+        await c.primary
+        self.assertIsNotNone(c._topology)
+        c = await self.async_rs_or_single_client(connect=False)
+        # secondaries causes client to block until connected
+        await c.secondaries
+        self.assertIsNotNone(c._topology)
+        c = await self.async_rs_or_single_client(connect=False)
+        # arbiters causes client to block until connected
+        await c.arbiters
+        self.assertIsNotNone(c._topology)
+        c = await self.async_rs_or_single_client(connect=False)
         # is_primary causes client to block until connected
         self.assertIsInstance(await c.is_primary, bool)
         c = await self.async_rs_or_single_client(connect=False)
@@ -2170,6 +2194,18 @@ async def test_uuid_queries(self):
         self.assertEqual(2, len(docs))
         await coll.drop()
 
+    async def test_unconnected_client_properties_with_srv(self):
+        client = self.simple_client("mongodb+srv://test1.test.build.10gen.cc/", connect=False)
+        self.assertEqual(client.nodes, frozenset())
+        topology_description = client.topology_description
+        self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown)
+        self.assertEqual(
+            topology_description.server_descriptions(),
+            {("unknown", None): ServerDescription(("unknown", None))},
+        )
+        await client.aconnect()
+        self.assertEqual(await client.address, None)
+
 
 class TestExhaustCursor(AsyncIntegrationTest):
     """Test that clients properly handle errors from exhaust cursors."""

test/test_client.py

@@ -791,6 +791,30 @@ def test_constants(self):
     def test_init_disconnected(self):
         host, port = client_context.host, client_context.port
         c = self.rs_or_single_client(connect=False)
+        # nodes returns an empty set if not connected
+        self.assertEqual(c.nodes, frozenset())
+        # topology_description returns the initial seed description if not connected
+        topology_description = c.topology_description
+        self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown)
+        self.assertEqual(
+            topology_description.server_descriptions(),
+            {(host, port): ServerDescription((host, port))},
+        )
+        # address causes client to block until connected
+        self.assertIsNotNone(c.address)
+        c = self.rs_or_single_client(connect=False)
+        # primary causes client to block until connected
+        c.primary
+        self.assertIsNotNone(c._topology)
+        c = self.rs_or_single_client(connect=False)
+        # secondaries causes client to block until connected
+        c.secondaries
+        self.assertIsNotNone(c._topology)
+        c = self.rs_or_single_client(connect=False)
+        # arbiters causes client to block until connected
+        c.arbiters
+        self.assertIsNotNone(c._topology)
+        c = self.rs_or_single_client(connect=False)
         # is_primary causes client to block until connected
         self.assertIsInstance(c.is_primary, bool)
         c = self.rs_or_single_client(connect=False)
@@ -2127,6 +2151,18 @@ def test_uuid_queries(self):
         self.assertEqual(2, len(docs))
         coll.drop()
 
+    def test_unconnected_client_properties_with_srv(self):
+        client = self.simple_client("mongodb+srv://test1.test.build.10gen.cc/", connect=False)
+        self.assertEqual(client.nodes, frozenset())
+        topology_description = client.topology_description
+        self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown)
+        self.assertEqual(
+            topology_description.server_descriptions(),
+            {("unknown", None): ServerDescription(("unknown", None))},
+        )
+        client._connect()
+        self.assertEqual(client.address, None)
+
 
 class TestExhaustCursor(IntegrationTest):
     """Test that clients properly handle errors from exhaust cursors."""