URL parsing fixes

Fixed a long-standing issue with :class:`.URL` where query parameters
following the question mark would not be parsed correctly if the URL did
not contain a database portion with a backslash.

Fixed issue where an ``@`` sign in the database portion of a URL would not
be interpreted correctly if the URL also had a username:password section.

Fixes: #6329
Fixes: #6482
Change-Id: I6cb6478affa49b618335b947a74e64090657a98c

diff --git a/doc/build/changelog/unreleased_14/6329.rst b/doc/build/changelog/unreleased_14/6329.rst
new file mode 100644 (file)
index 0000000..c71fed6
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6329.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, engine
+    :tickets: 6329
+
+    Fixed a long-standing issue with :class:`.URL` where query parameters
+    following the question mark would not be parsed correctly if the URL did
+    not contain a database portion with a backslash.

diff --git a/doc/build/changelog/unreleased_14/6482.rst b/doc/build/changelog/unreleased_14/6482.rst
new file mode 100644 (file)
index 0000000..5f3056e
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6482.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, engine
+    :tickets: 6482
+
+    Fixed issue where an ``@`` sign in the database portion of a URL would not
+    be interpreted correctly if the URL also had a username:password section.
+

diff --git a/lib/sqlalchemy/engine/url.py b/lib/sqlalchemy/engine/url.py
index 7898f912156db06e53ef559171edc2aa0774c629..f60b9d56d7a12ad47f5a658ac594c465d4b4ba53 100644 (file)
--- a/lib/sqlalchemy/engine/url.py
+++ b/lib/sqlalchemy/engine/url.py
@@ -702,16 +702,17 @@ def _parse_rfc1738_args(name):
             (?P<name>[\w\+]+)://
             (?:
                 (?P<username>[^:/]*)
-                (?::(?P<password>.*))?
+                (?::(?P<password>[^@]*))?
             @)?
             (?:
                 (?:
-                    \[(?P<ipv6host>[^/]+)\] |
-                    (?P<ipv4host>[^/:]+)
+                    \[(?P<ipv6host>[^/\?]+)\] |
+                    (?P<ipv4host>[^/:\?]+)
                 )?
-                (?::(?P<port>[^/]*))?
+                (?::(?P<port>[^/\?]*))?
             )?
-            (?:/(?P<database>.*))?
+            (?:/(?P<database>[^\?]*))?
+            (?:\?(?P<query>.*))?
             """,
         re.X,
     )
@@ -719,23 +720,17 @@ def _parse_rfc1738_args(name):
     m = pattern.match(name)
     if m is not None:
         components = m.groupdict()
-        if components["database"] is not None:
-            tokens = components["database"].split("?", 2)
-            components["database"] = tokens[0]
-
-            if len(tokens) > 1:
-                query = {}
-
-                for key, value in util.parse_qsl(tokens[1]):
-                    if util.py2k:
-                        key = key.encode("ascii")
-                    if key in query:
-                        query[key] = util.to_list(query[key])
-                        query[key].append(value)
-                    else:
-                        query[key] = value
-            else:
-                query = None
+        if components["query"] is not None:
+            query = {}
+
+            for key, value in util.parse_qsl(components["query"]):
+                if util.py2k:
+                    key = key.encode("ascii")
+                if key in query:
+                    query[key] = util.to_list(query[key])
+                    query[key].append(value)
+                else:
+                    query[key] = value
         else:
             query = None
         components["query"] = query

diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py
index 22853cf376d780aff0db9dfe1b5dcf9f425a5e4e..5e8fbfb5fa6a1209c4341d3d922c334c5b392b06 100644 (file)
--- a/test/engine/test_parseconnect.py
+++ b/test/engine/test_parseconnect.py
@@ -25,56 +25,60 @@ dialect = None
 
 
 class URLTest(fixtures.TestBase):
-    def test_rfc1738(self):
-        for text in (
-            "dbtype://username:password@hostspec:110//usr/db_file.db",
-            "dbtype://username:password@hostspec/database",
-            "dbtype+apitype://username:password@hostspec/database",
-            "dbtype://username:password@hostspec",
-            "dbtype://username:password@/database",
-            "dbtype://username@hostspec",
-            "dbtype://username:password@127.0.0.1:1521",
-            "dbtype://hostspec/database",
-            "dbtype://hostspec",
-            "dbtype://hostspec/?arg1=val1&arg2=val2",
-            "dbtype+apitype:///database",
-            "dbtype:///:memory:",
-            "dbtype:///foo/bar/im/a/file",
-            "dbtype:///E:/work/src/LEM/db/hello.db",
-            "dbtype:///E:/work/src/LEM/db/hello.db?foo=bar&hoho=lala",
-            "dbtype:///E:/work/src/LEM/db/hello.db?foo=bar&hoho=lala&hoho=bat",
-            "dbtype://",
-            "dbtype://username:password@/database",
-            "dbtype:////usr/local/_xtest@example.com/members.db",
-            "dbtype://username:apples%2Foranges@hostspec/database",
-            "dbtype://username:password@[2001:da8:2004:1000:202:116:160:90]"
-            "/database?foo=bar",
-            "dbtype://username:password@[2001:da8:2004:1000:202:116:160:90]:80"
-            "/database?foo=bar",
-        ):
-            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.host in (
-                "hostspec",
-                "127.0.0.1",
-                "2001:da8:2004:1000:202:116:160:90",
-                "",
-                None,
-            ), u.host
-            assert u.database in (
-                "database",
-                "/usr/local/_xtest@example.com/members.db",
-                "/usr/db_file.db",
-                ":memory:",
-                "",
-                "foo/bar/im/a/file",
-                "E:/work/src/LEM/db/hello.db",
-                None,
-            ), u.database
-            eq_(str(u), text)
+    @testing.combinations(
+        "dbtype://username:password@hostspec:110//usr/db_file.db",
+        "dbtype://username:password@hostspec/database",
+        "dbtype+apitype://username:password@hostspec/database",
+        "dbtype://username:password@hostspec",
+        "dbtype://username:password@/database",
+        "dbtype://username@hostspec",
+        "dbtype://username:password@127.0.0.1:1521",
+        "dbtype://hostspec/database",
+        "dbtype://hostspec",
+        "dbtype://hostspec/?arg1=val1&arg2=val2",
+        "dbtype+apitype:///database",
+        "dbtype:///:memory:",
+        "dbtype:///foo/bar/im/a/file",
+        "dbtype:///E:/work/src/LEM/db/hello.db",
+        "dbtype:///E:/work/src/LEM/db/hello.db?foo=bar&hoho=lala",
+        "dbtype:///E:/work/src/LEM/db/hello.db?foo=bar&hoho=lala&hoho=bat",
+        "dbtype://",
+        "dbtype://username:password@/database",
+        "dbtype:////usr/local/_xtest@example.com/members.db",
+        "dbtype://username:apples%2Foranges@hostspec/database",
+        "dbtype://username:password@[2001:da8:2004:1000:202:116:160:90]"
+        "/database?foo=bar",
+        "dbtype://username:password@[2001:da8:2004:1000:202:116:160:90]:80"
+        "/database?foo=bar",
+        "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",
+    )
+    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.host in (
+            "hostspec",
+            "127.0.0.1",
+            "2001:da8:2004:1000:202:116:160:90",
+            "",
+            None,
+        ), u.host
+        assert u.database in (
+            "database",
+            "test database with@atsign",
+            "/usr/local/_xtest@example.com/members.db",
+            "/usr/db_file.db",
+            ":memory:",
+            "",
+            "foo/bar/im/a/file",
+            "E:/work/src/LEM/db/hello.db",
+            None,
+        ), u.database
+        eq_(str(u), text)
 
     def test_rfc1738_password(self):
         u = url.make_url("dbtype://user:pass word + other%3Awords@host/dbname")
@@ -141,6 +145,23 @@ class URLTest(fixtures.TestBase):
         eq_(u.query, {"arg1": "param1", "arg2": "param2"})
         eq_(str(u), "dialect://user:pass@host/db?arg1=param1&arg2=param2")
 
+        u = url.make_url("dialect://user:pass@host/?arg1=param1&arg2=param2")
+        eq_(u.query, {"arg1": "param1", "arg2": "param2"})
+        eq_(u.database, "")
+        eq_(str(u), "dialect://user:pass@host/?arg1=param1&arg2=param2")
+
+        u = url.make_url("dialect://user:pass@host?arg1=param1&arg2=param2")
+        eq_(u.query, {"arg1": "param1", "arg2": "param2"})
+        eq_(u.database, None)
+        eq_(str(u), "dialect://user:pass@host?arg1=param1&arg2=param2")
+
+        u = url.make_url(
+            "dialect://user:pass@host:450?arg1=param1&arg2=param2"
+        )
+        eq_(u.port, 450)
+        eq_(u.query, {"arg1": "param1", "arg2": "param2"})
+        eq_(str(u), "dialect://user:pass@host:450?arg1=param1&arg2=param2")
+
         u = url.make_url(
             "dialect://user:pass@host/db?arg1=param1&arg2=param2&arg2=param3"
         )