Re: PQHost() undefined behavior if connecting string contains both host and hostaddr types

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: michael(at)paquier(dot)xyz
Cc: peter(dot)eisentraut(at)2ndquadrant(dot)com, kommi(dot)haribabu(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org, michael(dot)paquier(at)gmail(dot)com, robertmhaas(at)gmail(dot)com
Subject: Re: PQHost() undefined behavior if connecting string contains both host and hostaddr types
Date: 2018-03-16 04:03:14
Message-ID: 20180316.130314.19595957.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Fri, 16 Mar 2018 09:50:41 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20180316(dot)095041(dot)241173653(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> I drifted to come here..
>
> At Wed, 14 Mar 2018 11:17:35 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20180314021735(dot)GI1150(at)paquier(dot)xyz>
> > On Fri, Mar 09, 2018 at 04:42:30PM -0500, Peter Eisentraut wrote:
> > > It seems, however, that PGhost() has always been broken for hostaddr
> > > use. In 9.6 (before the multiple-hosts stuff was introduced), when
> > > connecting to "hostaddr=127.0.0.1", PGhost() returns "/tmp". Urgh.
> > >
> > > I think we should decide what PGhost() is supposed to mean when hostaddr
> > > is in use, and then make a fix for that consistently across all versions.
> >
> > Hm. The only inconsistency I can see here is when "host" is not defined
> > but "hostaddr" is, so why not make PQhost return the value of "hostaddr"
> > in this case?

I proposed that before and I strongly agree to that. I suppose
that the return value of PQhost is to used for the following
purpose.

- It is for authentication subject.
- It provides a string for a human to identify the connection peer.

So it is incovenient that PQhost returns pghost before trying
hostaddr. Attached patch does that based on Haribabu's patch at
the beginning of this thread.

================
> https://www.postgresql.org/message-id/19297.1493660213%40sss.pgh.pa.us
> > And I believe we already considered and rejected the idea of having it
> > return the hostaddr string, back in some of the older discussions.
> > (We could revisit that decision, no doubt, but let's go back and see
> > what the reasoning was first.)
>
> But I'havent find where the decision made. I'm going to search
> for that for a while.

After all, I didn't find much..

I found this thread back to 2014.

https://www.postgresql.org/message-id/CAHGQGwHsnMxh97UdXH5uif8eitD0WXDK_PSb3tipaGzJJVsCMw@mail.gmail.com

It preserved the behavior that PQhost() returns '/tmp' for the
case under discussion intentionally.

After that, 274bb2b385 (mistakenly?) put priority on hostaddr to
host in connectOptions2() so 11003eb556 reverted PQhost to return
pghost if connhost[] is of CHT_HOST_ADDRESS.

The last commit also reverted it to return /tmp if pghost is not
specified.

https://www.postgresql.org/message-id/CA+Tgmob1Y1KyK_Twi9oF3tj4p3J4c5YoNtAUh7DiajpeWynuLg@mail.gmail.com

I agree to the conclusion that PQhost() shouldn't return hostaddr
"if it has any host name to return". But I still haven't found
the reason for returning '/tmp' for IP connection.

The attached patch is revised version of that in the following thread.

https://www.postgresql.org/message-id/CA%2BTgmoZ%2B9h%3DmiD4%2BwYc9QztezgtLfeA59XtxVAL0NUjvfwKmaA%40mail.gmail.com
> Kyotaro Horiguchi argues that the current behavior is "not useful" and
> that's probably true, but I don't think it's the job of an API to try
> as hard as possible to do something useful in every case. It's more
> important that the behavior is predictable and understandable. In
> short, if we're going to change the behavior of PQhost() here, then
> there should be a documentation change to go with it, because the
> current documentation around the interaction between host and hostaddr
> does not make it at all clear that the current behavior is wrong, at
> least not as far as I can see. To the contrary, it suggests that if
> you use hostaddr without host, you may find the results surprising or
> even unfortunate:

I believe the behavior is not surprising and not a hard thing to do.

https://www.postgresql.org/message-id/20170510.153403.28896042.horiguchi.kyotaro%40lab.ntt.co.jp
> However, it might be a problem that the documentation doesn't
> mention what the returned value from PQhost is.

If it is the string that was given as host parameter, returning
"" instead of NULL might be reasonable. If it is any string that
points to the connecting host, I think that returning IP address
is not surprising at all.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
fix_pqhost_to_return_hostaddr_when_no_host_specification.patch text/x-patch 786 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2018-03-16 04:24:15 Re: PL/pgSQL nested CALL with transactions
Previous Message Michael Paquier 2018-03-16 03:51:04 Re: fixing more format truncation issues