Replace custom URL-encoding method with quote_plus

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_plus``, 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: I90b7a9f4dfdb719082b4b178ad4e009a8531a18e

diff --git a/doc/build/changelog/unreleased_21/10662.rst b/doc/build/changelog/unreleased_21/10662.rst
new file mode 100644 (file)
index 0000000..c5cc64a
--- /dev/null
+++ b/doc/build/changelog/unreleased_21/10662.rst
@@ -0,0 +1,9 @@
+.. 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_plus``, 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..bf1471a0fcb3beeae9e2988ee97a195481461e32 100644 (file)
--- a/lib/sqlalchemy/engine/url.py
+++ b/lib/sqlalchemy/engine/url.py
@@ -631,7 +631,7 @@ class URL(NamedTuple):
             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 +642,7 @@ class URL(NamedTuple):
             keys = list(self.query)
             keys.sort()
             s += "?" + "&".join(
-                "%s=%s" % (quote_plus(k), quote_plus(element))
+                f"{_sqla_url_quote(k)}={_sqla_url_quote(element)}"
                 for k in keys
                 for element in util.to_list(self.query[k])
             )
@@ -906,8 +906,7 @@ def _parse_url(name: str) -> URL:
         )
 
 
-def _sqla_url_quote(text: str) -> str:
-    return re.sub(r"[:@/]", lambda m: "%%%X" % ord(m.group(0)), text)
+_sqla_url_quote = quote_plus
 
 
 _sqla_url_unquote = unquote

diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py
index 4c144a4a31afde0dc3418a88dc605390d84266fc..846cd3b4defb106124750a5621f2d3f5c6743ac4 100644 (file)
--- a/test/engine/test_parseconnect.py
+++ b/test/engine/test_parseconnect.py
@@ -62,13 +62,19 @@ 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://user%25%26%7C:pass%25%26%7C@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%&|", None)
+        assert u.password in (
+            "password",
+            "apples/oranges",
+            "pass%&|",
+            None,
+        )
         assert u.host in (
             "hostspec",
             "127.0.0.1",
@@ -95,7 +101,7 @@ class URLTest(fixtures.TestBase):
         eq_(str(u), "dbtype://user:***@host/dbname")
         eq_(
             u.render_as_string(hide_password=False),
-            "dbtype://user:pass word + other%3Awords@host/dbname",
+            "dbtype://user:pass+word+%2B+other%3Awords@host/dbname",
         )
 
         u = url.make_url(