From 47656f1c91ee9c0fbd0942c8de840f9dec262588 Mon Sep 17 00:00:00 2001 From: arpitjain099 Date: Thu, 9 Jul 2026 10:13:00 +0900 Subject: [PATCH] Do not double-decode username and password in SQLAlchemy URL SQLAlchemy's make_url already percent-decodes the userinfo, so passing url.username and url.password through unquote_plus again corrupted any literal '+' into a space, breaking authentication for passwords that contain a plus sign. The form-encoding '+' means space convention only applies to the query string, not to the userinfo component, so the existing unquote_plus calls for query parameters are left unchanged. Fixes #611 Signed-off-by: arpitjain099 --- tests/unit/sqlalchemy/test_dialect.py | 13 +++++++++++++ trino/sqlalchemy/dialect.py | 8 ++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/unit/sqlalchemy/test_dialect.py b/tests/unit/sqlalchemy/test_dialect.py index d247a536..064faac1 100644 --- a/tests/unit/sqlalchemy/test_dialect.py +++ b/tests/unit/sqlalchemy/test_dialect.py @@ -271,6 +271,19 @@ def test_trino_connection_basic_auth(): assert cparams['auth']._password == password +def test_trino_connection_basic_auth_with_plus_in_credentials(): + # SQLAlchemy's make_url already percent-decodes the userinfo, so the dialect + # must not run it through unquote_plus again (that turns a literal '+' into a space). + dialect = TrinoDialect() + url = make_url("trino://user%2Bname:pass%2Bword@host") + _, cparams = dialect.create_connect_args(url) + + assert cparams['user'] == 'user+name' + assert isinstance(cparams['auth'], BasicAuthentication) + assert cparams['auth']._username == 'user+name' + assert cparams['auth']._password == 'pass+word' + + def test_trino_connection_jwt_auth(): dialect = TrinoDialect() access_token = 'sample-token' diff --git a/trino/sqlalchemy/dialect.py b/trino/sqlalchemy/dialect.py index ad05aeec..cbc44a17 100644 --- a/trino/sqlalchemy/dialect.py +++ b/trino/sqlalchemy/dialect.py @@ -127,13 +127,17 @@ def create_connect_args(self, url: URL) -> Tuple[Sequence[Any], Mapping[str, Any else: raise ValueError(f"Unexpected database format {url.database}") + # SQLAlchemy's make_url already percent-decodes the userinfo, so we use the + # username and password as-is. Running them through unquote_plus again would + # corrupt a literal '+' into a space (form-encoding only applies to the query + # string, not to the userinfo component). if url.username: - kwargs["user"] = unquote_plus(url.username) + kwargs["user"] = url.username if url.password: if not url.username: raise ValueError("Username is required when specify password in connection URL") - kwargs["auth"] = BasicAuthentication(unquote_plus(url.username), unquote_plus(url.password)) + kwargs["auth"] = BasicAuthentication(url.username, url.password) if "access_token" in url.query: kwargs["auth"] = JWTAuthentication(unquote_plus(url.query["access_token"]))