From: Mike Bayer Date: Sun, 1 Mar 2026 18:05:21 +0000 (-0500) Subject: make local mutable copies for cargs / cparams in do_connect X-Git-Tag: rel_2_0_48~2 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=c11ba33518979e27926abc26cdbe9b177d391a16;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 (cherry picked from commit dfb1c49cd7306eeca49fd7bb7ec4bcbef0e68d79) --- 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 9abb08f325..9ab94763b0 100644 --- a/lib/sqlalchemy/engine/create.py +++ b/lib/sqlalchemy/engine/create.py @@ -630,8 +630,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_tup, _cparams) = dialect.create_connect_args(u) + cparams = util.immutabledict(_cparams).union(pop_kwarg("connect_args", {})) if "async_fallback" in cparams and util.asbool(cparams["async_fallback"]): util.warn_deprecated( @@ -640,8 +640,6 @@ def create_engine(url: Union[str, _url.URL], **kwargs: Any) -> Engine: "2.0", ) - cargs = list(cargs_tup) # allow mutability - # look for existing pool or create pool = pop_kwarg("pool", None) if pool is None: @@ -650,15 +648,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 c0d80a738b..4c6bf8a28f 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -3897,6 +3897,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