|From:||Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>|
|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
- 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.
> > 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.
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
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.
> 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.
> 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.
NTT Open Source Software Center
|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|