From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sun, 1 Mar 2026 18:05:21 +0000 (-0500)
Subject: make local mutable copies for cargs / cparams in do_connect
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=94ed75eec98246c0c8a5cda8a82a779bd8b4f1c8;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

make local mutable copies for cargs / cparams in do_connect

Fixed a critical issue in :class:`.Engine` where connections created in
conjunction with the :meth:`.ConnectionEvents.do_connect` event listeners
would receive shared, mutable collections for the connection arguments,
leading to a variety of potential issues including unlimited growth of the
argument list as well as elements within the parameter dictionary being
shared among concurrent connection calls.  In particular this could impact
do_connect routines making use of complex mutable authentication
structures.

Fixes: #13144
Change-Id: I1549dae36e8e7e6cf50fdaf796659b53e7b78234
---

diff --git a/doc/build/changelog/unreleased_20/13144.rst b/doc/build/changelog/unreleased_20/13144.rst
new file mode 100644
index 0000000000..1514afc829
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/13144.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, engine
+    :tickets: 13144
+
+    Fixed a critical issue in :class:`.Engine` where connections created in
+    conjunction with the :meth:`.ConnectionEvents.do_connect` event listeners
+    would receive shared, mutable collections for the connection arguments,
+    leading to a variety of potential issues including unlimited growth of the
+    argument list as well as elements within the parameter dictionary being
+    shared among concurrent connection calls.  In particular this could impact
+    do_connect routines making use of complex mutable authentication
+    structures.
diff --git a/lib/sqlalchemy/engine/create.py b/lib/sqlalchemy/engine/create.py
index 324978202f..ee9251d0d1 100644
--- a/lib/sqlalchemy/engine/create.py
+++ b/lib/sqlalchemy/engine/create.py
@@ -614,9 +614,8 @@ def create_engine(url: Union[str, _url.URL], **kwargs: Any) -> Engine:
     dialect = dialect_cls(**dialect_args)
 
     # assemble connection arguments
-    (cargs_tup, cparams) = dialect.create_connect_args(u)
-    cparams.update(pop_kwarg("connect_args", {}))
-    cargs = list(cargs_tup)  # allow mutability
+    (cargs_tup, _cparams) = dialect.create_connect_args(u)
+    cparams = util.immutabledict(_cparams).union(pop_kwarg("connect_args", {}))
 
     # look for existing pool or create
     pool = pop_kwarg("pool", None)
@@ -626,15 +625,23 @@ def create_engine(url: Union[str, _url.URL], **kwargs: Any) -> Engine:
             connection_record: Optional[ConnectionPoolEntry] = None,
         ) -> DBAPIConnection:
             if dialect._has_events:
+                mutable_cargs = list(cargs_tup)
+                mutable_cparams = dict(cparams)
                 for fn in dialect.dispatch.do_connect:
                     connection = cast(
                         DBAPIConnection,
-                        fn(dialect, connection_record, cargs, cparams),
+                        fn(
+                            dialect,
+                            connection_record,
+                            mutable_cargs,
+                            mutable_cparams,
+                        ),
                     )
                     if connection is not None:
                         return connection
-
-            return dialect.connect(*cargs, **cparams)
+                return dialect.connect(*mutable_cargs, **mutable_cparams)
+            else:
+                return dialect.connect(*cargs_tup, **cparams)
 
         creator = pop_kwarg("creator", connect)
 
diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py
index 600cf10da9..98a42f11e9 100644
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -3955,6 +3955,54 @@ class DialectEventTest(fixtures.TestBase):
         with e.connect() as conn:
             eq_(conn.info["boom"], "one")
 
+    def test_connect_do_connect_no_cargs_cparams_leak_nullpool(self):
+        """test #13144, cargs/cparams in do_connect are not shared"""
+
+        e = engines.testing_engine(
+            options={"_initialize": False, "poolclass": NullPool}
+        )
+
+        m1 = Mock()
+        e.dialect.connect = m1.mock_connect
+
+        cargs_list = []
+        cparams_list = []
+        call_count = 0
+
+        @event.listens_for(e, "do_connect")
+        def evt(dialect, conn_rec, cargs, cparams):
+            nonlocal call_count
+            cargs_list.append(cargs)
+            cparams_list.append(cparams)
+
+            cargs.append(f"extra_arg_{call_count}")
+            cparams[f"extra_param_{call_count}"] = "value"
+            call_count += 1
+
+        for _ in range(3):
+            with e.connect():
+                pass
+
+        all_list_keys = ["extra_arg_0", "extra_arg_1", "extra_arg_2"]
+
+        for carg, expected in zip(cargs_list, all_list_keys):
+            eq_(set(carg).intersection(all_list_keys), {expected})
+
+        all_param_keys = ["extra_param_0", "extra_param_1", "extra_param_2"]
+        for cparam, expected in zip(cparams_list, all_param_keys):
+            eq_(set(cparam).intersection(all_param_keys), {expected})
+
+        eq_(
+            len(set(id(carg) for carg in cargs_list)),
+            3,
+            "cargs should be different objects",
+        )
+        eq_(
+            len(set(id(cparam) for cparam in cparams_list)),
+            3,
+            "cparams should be different objects",
+        )
+
 
 class SetInputSizesTest(fixtures.TablesTest):
     __backend__ = True