From: Mike Bayer Date: Fri, 7 Jul 2023 20:50:26 +0000 (-0400) Subject: if only one host and/or port in query string, never raise X-Git-Tag: rel_2_0_19~15 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fb060d99c6046ccde5946207233128415cb6ee8d;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git if only one host and/or port in query string, never raise silently ignore the host portion of the URL if host=xyz is present, and document this. Fixes: #10076 Change-Id: I370b138aec63a9a2aa6029fe9e66eb1659434a43 --- diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py index 5e0dee0ea3..d221574888 100644 --- a/lib/sqlalchemy/dialects/postgresql/base.py +++ b/lib/sqlalchemy/dialects/postgresql/base.py @@ -1405,9 +1405,11 @@ from collections import defaultdict from functools import lru_cache import re from typing import Any +from typing import cast from typing import List from typing import Optional from typing import Tuple +from typing import TYPE_CHECKING from typing import Union from . import array as _array @@ -3098,14 +3100,15 @@ class PGDialect(default.DefaultDialect): Tuple[None, None], Tuple[Tuple[Optional[str], ...], Tuple[Optional[int], ...]], ]: - hosts = ports = None + hosts: Optional[Tuple[Optional[str], ...]] = None + ports_str: Union[str, Tuple[Optional[str], ...], None] = None integrated_multihost = False if "host" in url.query: if isinstance(url.query["host"], (list, tuple)): integrated_multihost = True - hosts, ports = zip( + hosts, ports_str = zip( *[ token.split(":") if ":" in token else (token, None) for token in url.query["host"] @@ -3130,8 +3133,13 @@ class PGDialect(default.DefaultDialect): if host_port_match: integrated_multihost = True h, p = host_port_match.group(1, 2) + if TYPE_CHECKING: + assert isinstance(h, str) + assert isinstance(p, str) hosts = (h,) - ports = (p,) if p else (None,) + ports_str = cast( + "Tuple[Optional[str], ...]", (p,) if p else (None,) + ) if "port" in url.query: if integrated_multihost: @@ -3141,31 +3149,36 @@ class PGDialect(default.DefaultDialect): '"host=h1:p1&host=h2:p2&host=h3:p3" separately' ) if isinstance(url.query["port"], (list, tuple)): - ports = url.query["port"] + ports_str = url.query["port"] elif isinstance(url.query["port"], str): - ports = tuple(url.query["port"].split(",")) + ports_str = tuple(url.query["port"].split(",")) + + ports: Optional[Tuple[Optional[int], ...]] = None - if ports: + if ports_str: try: - ports = tuple(int(x) if x else None for x in ports) + ports = tuple(int(x) if x else None for x in ports_str) except ValueError: raise exc.ArgumentError( - f"Received non-integer port arguments: {ports}" + f"Received non-integer port arguments: {ports_str}" ) from None - if (hosts or ports) and url.host: - raise exc.ArgumentError( - "Can't combine fixed host and multihost URL formats" + if ports and ( + (not hosts and len(ports) > 1) + or ( + hosts + and ports + and len(hosts) != len(ports) + and (len(hosts) > 1 or len(ports) > 1) ) - - if ports and (not hosts or len(hosts) != len(ports)): + ): raise exc.ArgumentError("number of hosts and ports don't match") if hosts is not None: if ports is None: ports = tuple(None for _ in hosts) - return hosts, ports + return hosts, ports # type: ignore def do_begin_twophase(self, connection, xid): self.do_begin(connection.connection) diff --git a/lib/sqlalchemy/dialects/postgresql/psycopg2.py b/lib/sqlalchemy/dialects/postgresql/psycopg2.py index af1a33be06..27f63ea6e1 100644 --- a/lib/sqlalchemy/dialects/postgresql/psycopg2.py +++ b/lib/sqlalchemy/dialects/postgresql/psycopg2.py @@ -88,6 +88,7 @@ connection URI:: "postgresql+psycopg2://scott:tiger@192.168.0.199:5432/test?sslmode=require" ) + Unix Domain Connections ------------------------ @@ -104,6 +105,19 @@ using ``host`` as an additional keyword argument:: create_engine("postgresql+psycopg2://user:password@/dbname?host=/var/lib/postgresql") +.. warning:: The format accepted here allows for a hostname in the main URL + in addition to the "host" query string argument. **When using this URL + format, the initial host is silently ignored**. That is, this URL:: + + engine = create_engine("postgresql+psycopg2://user:password@myhost1/dbname?host=myhost2") + + Above, the hostname ``myhost1`` is **silently ignored and discarded.** The + host which is connected is the ``myhost2`` host. + + This is to maintain some degree of compatibility with PostgreSQL's own URL + format which has been tested to behave the same way and for which tools like + PifPaf hardcode two hostnames. + .. seealso:: `PQconnectdbParams \ diff --git a/test/dialect/postgresql/test_dialect.py b/test/dialect/postgresql/test_dialect.py index a55fc0a6bb..d94a2e2f60 100644 --- a/test/dialect/postgresql/test_dialect.py +++ b/test/dialect/postgresql/test_dialect.py @@ -470,6 +470,69 @@ class MultiHostConnectTest(fixtures.TestBase): " for asyncpg multiple host URL", }, ), + ( + # fixed host + multihost formats. + "postgresql+psycopg2://USER:PASS@hostfixed/DB?port=111", + { + "host": "hostfixed", + "port": "111", + "asyncpg_port": 111, + "dbname": "DB", + "user": "USER", + "password": "PASS", + }, + ), + ( + # fixed host + multihost formats. **silently ignore** + # the fixed host. See #10076 + "postgresql+psycopg2://USER:PASS@hostfixed/DB?host=hostA:111", + { + "host": "hostA", + "port": "111", + "asyncpg_port": 111, + "dbname": "DB", + "user": "USER", + "password": "PASS", + }, + ), + ( + # fixed host + multihost formats. **silently ignore** + # the fixed host. See #10076 + "postgresql+psycopg2://USER:PASS@hostfixed/DB" + "?host=hostA&port=111", + { + "host": "hostA", + "port": "111", + "asyncpg_port": 111, + "dbname": "DB", + "user": "USER", + "password": "PASS", + }, + ), + ( + # fixed host + multihost formats. **silently ignore** + # the fixed host. See #10076 + "postgresql+psycopg2://USER:PASS@hostfixed/DB?host=hostA", + { + "host": "hostA", + "dbname": "DB", + "user": "USER", + "password": "PASS", + }, + ), + ( + # fixed host + multihost formats. if there is only one port + # or only one host after the query string, assume that's the + # host/port + "postgresql+psycopg2://USER:PASS@/DB?port=111", + { + "dbname": "DB", + "user": "USER", + "password": "PASS", + "port": "111", + "asyncpg_port": 111, + }, + ), ] for url_string, expected_psycopg in psycopg_combinations: asyncpg_error = expected_psycopg.pop("asyncpg_error", False) @@ -559,39 +622,12 @@ class MultiHostConnectTest(fixtures.TestBase): ): dialect.create_connect_args(u) - @testing.combinations( - ("postgresql+psycopg2://USER:PASS@hostfixed/DB?port=111",), - ("postgresql+psycopg2://USER:PASS@hostfixed/DB?host=hostA:111",), - ( - "postgresql+psycopg2://USER:PASS@hostfixed/DB" - "?host=hostA&port=111", - ), - ("postgresql+psycopg2://USER:PASS@hostfixed/DB" "?host=hostA",), - argnames="url_string", - ) - @testing.combinations( - psycopg2_dialect.dialect(), - psycopg_dialect.dialect(), - asyncpg_dialect.dialect(), - argnames="dialect", - ) - def test_dont_use_fixed_host(self, dialect, url_string): - url_string = url_string.replace("psycopg2", dialect.driver) - - u = url.make_url(url_string) - with expect_raises_message( - exc.ArgumentError, - "Can't combine fixed host and multihost URL formats", - ): - dialect.create_connect_args(u) - @testing.combinations( ( "postgresql+psycopg2://USER:PASS@/DB" "?host=hostA,hostC&port=111,222,333", ), ("postgresql+psycopg2://USER:PASS@/DB" "?host=hostA&port=111,222",), - ("postgresql+psycopg2://USER:PASS@/DB?port=111",), ( "postgresql+asyncpg://USER:PASS@/DB" "?host=hostA,hostB,hostC&port=111,333",