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)

diff --git a/doc/build/changelog/unreleased_20/10662.rst b/doc/build/changelog/unreleased_20/10662.rst
new file mode 100644 (file)
index 0000000..5be613d
--- /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 5cf5ec7b4b7df72078eb975534388d24d8888fa6..04ae5e91fbb13ee4a7818bc3291cf23a25fc38de 100644 (file)
--- 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 4c144a4a31afde0dc3418a88dc605390d84266fc..34dc1d7aa82833786392c58e46183c0ee5b0f749 100644 (file)
--- 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")