Re: libpq host/hostaddr/conninfo inconsistencies

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: libpq host/hostaddr/conninfo inconsistencies
Date: 2018-10-23 07:21:00
Message-ID: alpine.DEB.2.21.1810221724050.3819@lancre
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers


Hello Arthur,

>>>> sh> psql "host=127.0.0.2 hostaddr=127.0.0.1"
>>>
>>> I'm not sure that is is the issue. User defined the host name and psql
>>> show it.

>> The issue is that "host" is an ip, "\conninfo" will inform wrongly that you
>> are connected to "127.0.0.2", but the actual connection is really to
>> "127.0.0.1", this is plain misleading, and I consider this level of
>> unhelpfullness more a bug than a feature.
>
> I didn't think that this is an issue, because I determined "host" as just a
> host's display name when "hostaddr" is defined.

When I type "\conninfo", I do not expect to have false clues that must be
interpreted depending on a fine knowledge of the documentation and the
connection parameters possibly typed hours earlier, I would just expect to
have a direct answer describing in a self contained way what the
connection actually is.

> So user may determine 127.0.0.1 (hostaddr) as "happy_host", for example.
> It shouldn't be a real host.

They may determine it if they can access the initial connection
information, which means an careful inquest because \conninfo does not say
what it is... If they just read what is said, they just get wrong
informations.

> I searched for another use cases of PQhost(). In PostgreSQL source code I
> found that it is used in pg_dump and psql to connect to some instance.

> There is the next issue with PQhost() and psql (pg_dump could have it too,
> see CloneArchive() in pg_backup_archiver.c and _connectDB() in
> pg_backup_db.c):
>
> $ psql "host=host_1,host_2 hostaddr=127.0.0.1,127.0.0.3 dbname=postgres"
> =# \conninfo
> You are connected to database "postgres" as user "artur" on host "host_1" at
> port "5432".
> =# \connect test
> could not translate host name "host_1" to address: Неизвестное имя или служба
> Previous connection kept
>
> So in the example above you cannot reuse connection string with \connect.
> What do you think?

I think that this is another connection related "feature", aka bug, that
should be fixed as well:-(

>>> I cannot agree with you. When I've learned libpq before I found
>>> host/hostaddr rules description useful. And I disagree that it is good
>>> to remove it (as the patch does).

>>> Of course it is only my point of view and others may have another opinion.
>>
>> I'm not sure I understand your concern.
>>
>> Do you mean that you would prefer the document to keep describing that
>> host/hostaddr/port accepts one value, and then have in some other place or
>> at the end of the option documentation a line that say, "by the way, we
>> really accept lists, and they must be somehow consistent between
>> host/hostaddr/port"?
>
> I wrote about the following part of the documentation:
>
>> - Using <literal>hostaddr</literal> instead of <literal>host</literal> allows the
>> - application to avoid a host name look-up, which might be important
>> - in applications with time constraints. However, a host name is
>> - required for GSSAPI or SSPI authentication
>> - methods, as well as for <literal>verify-full</literal> SSL
>> - certificate verification. The following rules are used:
>> - <itemizedlist>
>> ...

> So I think description of these rules is useful here and shouldn't be
> removed.

Ok, I have put back a summary description of which rules apply, which are
somehow simpler & saner, at least this is the aim of this patch.

> Your patch removes it and maybe it shouldn't do that. But now I
> realised that the patch breaks this behavior and backward compatibility
> is broken.

Indeed. The incompatible changes are that "host" must always be provided,
instead of letting the user providing an IP either in host or hostaddr
(currently both work although undocumented), and that "hostaddr" can only
be provided for a host name, not for an IP or socket.

>>> Patch gives me an error if I specified only hostaddr:
>>>
>>> psql -d "hostaddr=127.0.0.1"
>>> psql: host "/tmp" cannot have an hostaddr "127.0.0.1"
>>
>> This is the expected modified behavior: hostaddr can only be specified on a
>> host when it is a name, which is not the case here.
>
> See the comment above about backward compatibility. psql without the patch
> can connect to an instance if I specify only hostaddr.

Yes, that is intentional and is the purpose of this patch: to provide a
simple connection model for the user: use "host" to connect to a target
server, and "hostaddr" as a lookup shortcut only.

For a reminder, my main issues with the current status are:

(1) the documentation is inconsistent with the implementation:
"host" can be given an IP, but this is not documented.
"hostaddr" can be provided for anything, and overshadows the initial
specification, but:

(2) "\conninfo" does not give a clue about what the connection
really is in such cases.

Moreover, you found another issue with psql's "\connect" which does not
work properly when both "host" & "hostaddr" are given.

In the attached patch, I tried to clarify the documentation further and
fix some rebase issues I had. ISTM that all relevant informations provided
in the previous version are still there.

The backward incompatibility is clearly documented.

The patch does not address the \conninfo issue, which requires extending
libpq. I think that the \connect issue you raised is linked to the same
set of problems within libpq, which does not provide any reliable way to
know about the current connection in some cases, either for describing it
or reusing it.

--
Fabien.

Attachment Content-Type Size
libpq-host-ip-3.patch text/x-diff 19.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Laurenz Albe 2018-10-23 07:42:16 Re: Function to promote standby servers
Previous Message Haribabu Kommi 2018-10-23 07:11:51 Re: Pluggable Storage - Andres's take