From 64a5a2fa37815508ddb54ebf34740c141cb65226 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 6 Dec 2023 14:10:28 -0500 Subject: [PATCH] Replace custom URL-encoding method with quote Fixed URL-encoding of the username and password components of :class:`.engine.URL` objects when converting them to string using the :meth:`_engine.URL.render_as_string` method, by using Python standard library ``urllib.parse.quote`` while allowing for plus signs and spaces to remain unchanged as supported by SQLAlchemy's non-standard URL parsing, rather than the legacy home-grown routine from many years ago. Pull request courtesy of Xavier NUNN. Fixes: #10662 Closes: #10726 Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/10726 Pull-request-sha: 82219041b8f73d8c932cc40e87c002b3b853e02e Change-Id: Iedca4929579d4d26ef8cce083252dcd1e476286b (cherry picked from commit 4438883c9703affa3f441be9a230a5f751905a05) --- doc/build/changelog/unreleased_20/10662.rst | 11 +++++++++ lib/sqlalchemy/engine/url.py | 20 ++++++--------- test/engine/test_parseconnect.py | 27 ++++++++++++++++++--- 3 files changed, 42 insertions(+), 16 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/10662.rst diff --git a/doc/build/changelog/unreleased_20/10662.rst b/doc/build/changelog/unreleased_20/10662.rst new file mode 100644 index 0000000000..5be613d8e2 --- /dev/null +++ b/doc/build/changelog/unreleased_20/10662.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, engine + :tickets: 10662 + + Fixed URL-encoding of the username and password components of + :class:`.engine.URL` objects when converting them to string using the + :meth:`_engine.URL.render_as_string` method, by using Python standard + library ``urllib.parse.quote`` while allowing for plus signs and spaces to + remain unchanged as supported by SQLAlchemy's non-standard URL parsing, + rather than the legacy home-grown routine from many years ago. Pull request + courtesy of Xavier NUNN. diff --git a/lib/sqlalchemy/engine/url.py b/lib/sqlalchemy/engine/url.py index 5cf5ec7b4b..04ae5e91fb 100644 --- a/lib/sqlalchemy/engine/url.py +++ b/lib/sqlalchemy/engine/url.py @@ -32,6 +32,7 @@ from typing import Tuple from typing import Type from typing import Union from urllib.parse import parse_qsl +from urllib.parse import quote from urllib.parse import quote_plus from urllib.parse import unquote @@ -621,17 +622,17 @@ class URL(NamedTuple): """ s = self.drivername + "://" if self.username is not None: - s += _sqla_url_quote(self.username) + s += quote(self.username, safe=" +") if self.password is not None: s += ":" + ( "***" if hide_password - else _sqla_url_quote(str(self.password)) + else quote(str(self.password), safe=" +") ) s += "@" if self.host is not None: if ":" in self.host: - s += "[%s]" % self.host + s += f"[{self.host}]" else: s += self.host if self.port is not None: @@ -642,7 +643,7 @@ class URL(NamedTuple): keys = list(self.query) keys.sort() s += "?" + "&".join( - "%s=%s" % (quote_plus(k), quote_plus(element)) + f"{quote_plus(k)}={quote_plus(element)}" for k in keys for element in util.to_list(self.query[k]) ) @@ -885,10 +886,10 @@ def _parse_url(name: str) -> URL: components["query"] = query if components["username"] is not None: - components["username"] = _sqla_url_unquote(components["username"]) + components["username"] = unquote(components["username"]) if components["password"] is not None: - components["password"] = _sqla_url_unquote(components["password"]) + components["password"] = unquote(components["password"]) ipv4host = components.pop("ipv4host") ipv6host = components.pop("ipv6host") @@ -904,10 +905,3 @@ def _parse_url(name: str) -> URL: raise exc.ArgumentError( "Could not parse SQLAlchemy URL from string '%s'" % name ) - - -def _sqla_url_quote(text: str) -> str: - return re.sub(r"[:@/]", lambda m: "%%%X" % ord(m.group(0)), text) - - -_sqla_url_unquote = unquote diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py index 4c144a4a31..34dc1d7aa8 100644 --- a/test/engine/test_parseconnect.py +++ b/test/engine/test_parseconnect.py @@ -62,13 +62,33 @@ class URLTest(fixtures.TestBase): "dbtype://username:password@hostspec/test database with@atsign", "dbtype://username:password@hostspec?query=but_no_db", "dbtype://username:password@hostspec:450?query=but_no_db", + "dbtype://username:password with spaces@hostspec:450?query=but_no_db", + "dbtype+apitype://username with space+and+plus:" + "password with space+and+plus@" + "hostspec:450?query=but_no_db", + "dbtype://user%25%26%7C:pass%25%26%7C@hostspec:499?query=but_no_db", + "dbtype://user🐍測試:pass🐍測試@hostspec:499?query=but_no_db", ) def test_rfc1738(self, text): u = url.make_url(text) assert u.drivername in ("dbtype", "dbtype+apitype") - assert u.username in ("username", None) - assert u.password in ("password", "apples/oranges", None) + assert u.username in ( + "username", + "user%&|", + "username with space+and+plus", + "user🐍測試", + None, + ) + assert u.password in ( + "password", + "password with spaces", + "password with space+and+plus", + "apples/oranges", + "pass%&|", + "pass🐍測試", + None, + ) assert u.host in ( "hostspec", "127.0.0.1", @@ -87,7 +107,8 @@ class URLTest(fixtures.TestBase): "E:/work/src/LEM/db/hello.db", None, ), u.database - eq_(u.render_as_string(hide_password=False), text) + + eq_(url.make_url(u.render_as_string(hide_password=False)), u) def test_rfc1738_password(self): u = url.make_url("dbtype://user:pass word + other%3Awords@host/dbname") -- 2.47.2