From: Gord Thompson <gord@gordthompson.com>
Date: Tue, 9 Apr 2024 19:13:52 +0000 (-0600)
Subject: Avoid removing + from odbc_connect parameter values
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4a108c43d98a56e2f2e8db85d79e43b71ae96b8f;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Avoid removing + from odbc_connect parameter values

Fixes #11250

Change-Id: I2680923d366030343b7695a1a9072352134db8d7
---

diff --git a/doc/build/changelog/migration_21.rst b/doc/build/changelog/migration_21.rst
index 8d000e6033..45a152c7b3 100644
--- a/doc/build/changelog/migration_21.rst
+++ b/doc/build/changelog/migration_21.rst
@@ -134,3 +134,26 @@ lambdas which do the same::
 
 :ticket:`10050`
 
+
+.. _change_11250:
+
+Potential breaking change to odbc_connect= handling for mssql+pyodbc
+--------------------------------------------------------------------
+
+Fixed a mssql+pyodbc issue where valid plus signs in an already-unquoted
+``odbc_connect=`` (raw DBAPI) connection string were replaced with spaces.
+
+Previously, the pyodbc connector would always pass the odbc_connect value
+to unquote_plus(), even if it was not required. So, if the (unquoted)
+odbc_connect value contained ``PWD=pass+word`` that would get changed to
+``PWD=pass word``, and the login would fail. One workaround was to quote
+just the plus sign — ``PWD=pass%2Bword`` — which would then get unquoted
+to ``PWD=pass+word``.
+
+Implementations using the above workaround with :meth:`_engine.URL.create`
+to specify a plus sign in the ``PWD=`` argument of an odbc_connect string
+will have to remove the workaround and just pass the ``PWD=`` value as it
+would appear in a valid ODBC connection string (i.e., the same as would be
+required if using the connection string directly with ``pyodbc.connect()``).
+
+:ticket:`11250`
diff --git a/doc/build/changelog/unreleased_21/11250.rst b/doc/build/changelog/unreleased_21/11250.rst
new file mode 100644
index 0000000000..ba1fc14b73
--- /dev/null
+++ b/doc/build/changelog/unreleased_21/11250.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, mssql
+    :tickets: 11250
+
+    Fix mssql+pyodbc issue where valid plus signs in an already-unquoted
+    ``odbc_connect=`` (raw DBAPI) connection string are replaced with spaces.
+
+    The pyodbc connector would unconditionally pass the odbc_connect value
+    to unquote_plus(), even if it was not required. So, if the (unquoted)
+    odbc_connect value contained ``PWD=pass+word`` that would get changed to
+    ``PWD=pass word``, and the login would fail. One workaround was to quote
+    just the plus sign — ``PWD=pass%2Bword`` — which would then get unquoted
+    to ``PWD=pass+word``.
diff --git a/lib/sqlalchemy/connectors/pyodbc.py b/lib/sqlalchemy/connectors/pyodbc.py
index f204d80a8e..d2df4b9ed0 100644
--- a/lib/sqlalchemy/connectors/pyodbc.py
+++ b/lib/sqlalchemy/connectors/pyodbc.py
@@ -16,7 +16,6 @@ from typing import List
 from typing import Optional
 from typing import Tuple
 from typing import Union
-from urllib.parse import unquote_plus
 
 from . import Connector
 from .. import ExecutionContext
@@ -75,7 +74,8 @@ class PyODBCConnector(Connector):
                 connect_args[param] = util.asbool(keys.pop(param))
 
         if "odbc_connect" in keys:
-            connectors = [unquote_plus(keys.pop("odbc_connect"))]
+            # (potential breaking change for issue #11250)
+            connectors = [keys.pop("odbc_connect")]
         else:
 
             def check_quote(token: str) -> str:
diff --git a/test/dialect/mssql/test_engine.py b/test/dialect/mssql/test_engine.py
index e87b9825f1..557341aa6a 100644
--- a/test/dialect/mssql/test_engine.py
+++ b/test/dialect/mssql/test_engine.py
@@ -216,6 +216,55 @@ class ParseConnectTest(fixtures.TestBase):
             connection,
         )
 
+    @testing.combinations(
+        (
+            "quoted_plus",
+            (
+                "mssql+pyodbc:///?odbc_connect=DSN%3Dmydsn%3B"
+                "UID%3Ded%3BPWD%3Dpass%2Bword"
+            ),
+            "DSN=mydsn;UID=ed;PWD=pass+word",
+            ("DSN=mydsn;UID=ed;PWD=pass+word",),
+            "",
+        ),
+        (
+            "plus_for_space",
+            (
+                "mssql+pyodbc:///?odbc_connect=DSN%3Dmydsn%3B"
+                "UID%3Ded%3BPWD%3Dpass+word"
+            ),
+            "DSN=mydsn;UID=ed;PWD=pass word",
+            ("DSN=mydsn;UID=ed;PWD=pass word",),
+            "",
+        ),
+        (
+            "issue_11250_breaking_change",
+            (
+                "mssql+pyodbc:///?odbc_connect=DSN%3Dmydsn%3B"
+                "UID%3Ded%3BPWD%3Dpass%252Bword"
+            ),
+            "DSN=mydsn;UID=ed;PWD=pass%2Bword",
+            ("DSN=mydsn;UID=ed;PWD=pass%2Bword",),
+            "pre-11250 would unquote_plus() to PWD=pass+word",
+        ),
+        argnames="quoted_url, value_in_url_object, connection_string",
+        id_="iaaai",
+    )
+    def test_pyodbc_odbc_connect_with_pwd_plus(
+        self, quoted_url, value_in_url_object, connection_string
+    ):
+        dialect = pyodbc.dialect()
+        u = url.make_url(quoted_url)
+        eq_(value_in_url_object, u.query["odbc_connect"])
+        connection = dialect.create_connect_args(u)
+        eq_(
+            (
+                (connection_string),
+                {},
+            ),
+            connection,
+        )
+
     def test_pyodbc_odbc_connect_ignores_other_values(self):
         dialect = pyodbc.dialect()
         u = url.make_url(