pg_service.conf file with unknown parameters

On Fri, Sep 11, 2020 at 02:39:51PM +0200, Magnus Hagander wrote:

Right now, pg_service.conf returns "syntax error" if it encounters a parameter
it doesn't know about.

This seems user-unfriendly, both in the error message (because it really isn't
a syntax error) and in the behaviour itself (because it doesn't work when
sometimes it should).

For example, if I have a service file with gssencmode=disable set, that service
file cannot be used by a psql client linked against libpq from version 10. Even
if the behavior would be identical (since v10 doesn't have gssencmode).

Is there a particular reason we (1) refuse unknown parameters in the file, and
(2) call it a "syntax error"?

The documentation just says it's "INI format" file -- though in my experience
most other INI file parsers just ignore extra parameters included..

My guess is that because the file can contain passwords, we want to
report as little as possible on error.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

The usefulness of a cup is in its emptiness, Bruce Lee

On 11 Sep 2020, at 14:39, Magnus Hagander <magnus@hagander.net> wrote:

For example, if I have a service file with gssencmode=disable set, that service file cannot be used by a psql client linked against libpq from version 10. Even if the behavior would be identical (since v10 doesn't have gssencmode).

Is there a particular reason we (1) refuse unknown parameters in the file,

The above case is a good example when silently ignoring would be beneficial.
We would however run the risk that someone has this in their service which is
silently ignored and fails to notice:

ssl_mim_protocol_version=TLSv1.3

Not sure if that's worth the risk? Halting on unknown parameters is also
consistent with postgresql.conf parsing etc (which isn't a clientside file I
know, but still relevant IMO).

and (2) call it a "syntax error"?

That I agree with isn't entirely correct, the syntax is correct but the
parameter is unknown. Something along the following seems more correct:

  -  libpq_gettext("syntax error in service file \"%s\", line %d\n"),
  +  libpq_gettext("unknown parameter in service file \"%s\", line %d\n"),

The documentation just says it's "INI format" file -- though in my experience most other INI file parsers just ignore extra parameters included..

I don't think the INI file format is formally defined anywhere, but I do
believe it's considered to be strictly key-values (Wikipedia [0]https://en.wikipedia.org/wiki/INI_file agrees with
that). Since we allow ldap configurations like the one below, it's technically
not INI format though:

[service=mydb]
ldap://127.0.0.1:10000/foo?bar?lorem?ipsum

That might be borderline hairsplitting, but perhaps calling it INI format in
the docs isn't really helping? Maybe we should reword that to say key/value or
something similar?

And this brings up an even more interesting case, the above will yield a syntax
error if libpq wasn't compiled with LDAP support, as opposed to other
parameters (like SSL* etc) which are ignored for builds not supporting them.
Is there a reason to treat ldap differently?

cheers ./daniel

[0]: https://en.wikipedia.org/wiki/INI_file