pgcli icon indicating copy to clipboard operation
pgcli copied to clipboard

pgcli couldn't parse password that contained character '#'

Open leafonsword opened this issue 7 months ago • 2 comments

I have a section that contains character '#' in ~/.pg_service.conf, like

[staging_instance]
host=XXXX
port=5432
dbname=postgres
user=root
password=abc#123

using psql could login successfully

~# psql service=staging_instance
psql (13.20, server 15.12)
WARNING: psql major version 13, server major version 15.
         Some psql features might not work.
Type "help" for help.

postgres=>

but using pgcli still prompt input password

~# pgcli service=staging_instance
Password for root:

How could solve this?

leafonsword avatar May 29 '25 03:05 leafonsword

This is due to pgcli using the configobj library to load .pg_service.conf file. This library interprets "password=abc#123" as the value abc followed by an inline comment. psql does not do the same and sees abc#123 as the password.

For configobj (and thus the current version of pgcli) to correctly parse the password, one could enclose the password in quotes in .pg_service.conf, like this: password="abc#123".

But then the file would not be compatible with psql anymore, since psql would see the password as "abc#123" with the quotes.

The configparser standard Python module does not support inline comments by default (see documentation here). So if pgcli used it to parse .pg_service_conf ,pgcli would behave like psql. We could use it to parse .pg_service.conf, something like this:

-    with open(service_file, newline="") as f:
-        skipped_lines = skip_initial_comment(f)
-        try:
-            service_file_config = ConfigObj(f)
-        except ParseError as err:
-            err.line_number += skipped_lines
-            raise err
-    if service not in service_file_config:
+    service_file_config = configparser.ConfigParser()
+    service_file_config.read(service_file)
+    try:
+        section = service_file_config[service]
+    except KeyError:
         return None, service_file
-    service_conf = service_file_config.get(service)
-    return service_conf, service_file
+    return dict(section), service_file

(It's a quick-and-dirty and not thoroughly tested patch, but it seems to work.)

Pros: it makes pgcli compatible with psql way of parsing .pg_service.conf. Cons: it breaks pgcli's current behaviour for users who quotes password (or other values) in that file.

I think that a better feature parity with psql is worth this (small) breaking change (which can be documented in the changelog and easily fixed by users). @amjith and @j-bennet : what's your take on this? If you agree, I'll make a bit more tests and create a pull request.

dbaty avatar May 29 '25 08:05 dbaty

@dbaty Thanks for explaination, I think using configparser is a better solution that compatible with psql.

leafonsword avatar May 30 '25 05:05 leafonsword