From d3648deceaa02da7e621b65c0843fe4b98331d0f Mon Sep 17 00:00:00 2001 From: Diego Date: Tue, 18 Aug 2026 14:32:43 -0300 Subject: [PATCH] Keep the connection string when using -l/--list or --ping `pgcli "postgresql://user@host:5432/db?sslmode=verify-ca" -l` fails with `role "" does not exist` on a local socket, while the same connection string without -l connects fine. cli() replaces the positional argument with "postgres" whenever --list/--ping is given, on the grounds that those options do not take a db name. But a connection string is not a db name: a URI or a key=value conninfo carries the whole connection (host, user, port, sslmode, ...), and discarding it leaves pgcli with no connection details at all, so it falls back to a local socket connection as the OS user. Only a plain database name is discarded now. A connection string is passed through untouched, and if it names no database, "postgres" is merged in for the listing, since libpq would otherwise default to the OS user name, which is rarely an existing database. Adds five tests covering both connection-string forms with -l and --ping, the missing-dbname fallback, and the unchanged plain-db-name behaviour. --- changelog.rst | 6 +++++ pgcli/main.py | 18 +++++++++++-- tests/test_main.py | 65 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/changelog.rst b/changelog.rst index 6e4c47c36..9eaeb99e6 100644 --- a/changelog.rst +++ b/changelog.rst @@ -3,6 +3,12 @@ Upcoming (TBD) Bug fixes: ---------- +* Fix ``-l``/``--list`` and ``--ping`` discarding the connection string. The + positional argument was unconditionally replaced with ``postgres``, which + also threw away a connection URI or ``key=value`` conninfo (host, user, port, + ``sslmode``, everything) and silently fell back to a local socket connection + as the OS user. Only a plain database name is discarded now; a connection + string that names no database gets ``postgres`` for the listing. * Restore cursor shape behaviour for Emacs mode * Fix ``TypeError: cannot use a string pattern on a bytes-like object`` when completion metadata comes back as bytes (e.g. ``SQL_ASCII`` client encoding). diff --git a/pgcli/main.py b/pgcli/main.py index e3ba5bfa8..d93813e69 100644 --- a/pgcli/main.py +++ b/pgcli/main.py @@ -1634,9 +1634,23 @@ def cli( service = database[8:] elif os.getenv("PGSERVICE") is not None: service = os.getenv("PGSERVICE") - # because option --ping, --list or -l are not supposed to have a db name + # because option --ping, --list or -l are not supposed to have a db name. + # A connection string is not a db name though: a URI or a key=value conninfo + # carries the whole connection (host, user, port, sslmode, ...), so replacing + # it with "postgres" would throw all of that away and fall back to a local + # socket connection as the OS user. Only a plain db name is discarded here; + # a connection string that names no database gets "postgres" for the + # listing, since libpq would otherwise default to the OS user name. + is_conn_string = "://" in database or ("=" in database and service is None) if list_databases or ping_database: - database = "postgres" + if not is_conn_string: + database = "postgres" + else: + try: + if not conninfo_to_dict(database).get("dbname"): + database = make_conninfo(database, dbname="postgres") + except Exception: + pass # invalid conninfo: let the connection attempt report it cfg = load_config(pgclirc, config_full_path) if dsn != "": diff --git a/tests/test_main.py b/tests/test_main.py index ba990631a..5595c504d 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -6,6 +6,7 @@ from unittest import mock import pytest +from click.testing import CliRunner try: import setproctitle @@ -13,6 +14,7 @@ setproctitle = None from pgcli.main import ( + cli, obfuscate_process_password, duration_in_words, format_output, @@ -23,6 +25,7 @@ COLOR_CODE_REGEX, ) from pgcli.pgexecute import PGExecute +from psycopg.conninfo import conninfo_to_dict from pgspecial.main import PAGER_OFF, PAGER_LONG_OUTPUT, PAGER_ALWAYS from utils import dbtest, run from collections import namedtuple @@ -701,3 +704,65 @@ def test_get_editor_precedence(): # Nothing set -> None, so click uses its platform default. with mock.patch.dict(os.environ, {}, clear=True): assert get_editor() is None + + +def _cli_conn_target(argv, tmpdir): + """Run cli() with argv and report which connect_* path it took.""" + rc = tmpdir.join("rcfile") + rc.write("[main]\n") + runner = CliRunner() + with ( + mock.patch.object(PGCli, "connect_uri", side_effect=RuntimeError("stop")) as mock_uri, + mock.patch.object(PGCli, "connect_dsn", side_effect=RuntimeError("stop")) as mock_dsn, + mock.patch.object(PGCli, "connect", side_effect=RuntimeError("stop")) as mock_plain, + ): + runner.invoke(cli, argv + ["--pgclirc", str(rc)]) + if mock_uri.called: + return "uri", mock_uri.call_args + if mock_dsn.called: + return "dsn", mock_dsn.call_args + if mock_plain.called: + return "plain", mock_plain.call_args + return "none", None + + +def test_list_databases_keeps_uri(tmpdir): + """-l must not discard a connection URI: doing so fell back to a local + socket connection as the OS user.""" + uri = "postgresql://someuser@somehost:6000/somedb" + path, call = _cli_conn_target([uri, "-l"], tmpdir) + assert path == "uri" + assert call.args[0] == uri + + +def test_list_databases_keeps_kv_conninfo(tmpdir): + """Same for a key=value conninfo string, which carries sslmode and friends.""" + kv = "host=somehost port=6000 user=someuser dbname=somedb sslmode=verify-ca" + path, call = _cli_conn_target([kv, "-l"], tmpdir) + assert path == "dsn" + assert call.args[0] == kv + + +def test_ping_keeps_uri(tmpdir): + """--ping handles connection strings the same way as -l.""" + uri = "postgresql://someuser@somehost:6000/somedb" + path, call = _cli_conn_target([uri, "--ping"], tmpdir) + assert path == "uri" + assert call.args[0] == uri + + +def test_list_databases_conn_string_without_dbname_gets_postgres(tmpdir): + """A connection string naming no database gets "postgres" for the listing, + instead of libpq defaulting to the OS user name.""" + kv = "host=somehost user=someuser sslmode=verify-ca" + path, call = _cli_conn_target([kv, "-l"], tmpdir) + assert path == "dsn" + assert conninfo_to_dict(call.args[0])["dbname"] == "postgres" + assert conninfo_to_dict(call.args[0])["sslmode"] == "verify-ca" # rest preserved + + +def test_list_databases_discards_plain_dbname(tmpdir): + """A plain db name is still discarded by -l.""" + path, call = _cli_conn_target(["mydb", "-l"], tmpdir) + assert path == "plain" + assert call.args[0] == "postgres"