From: Tom Lane <tgl@sss.pgh.pa.us>
To: Haribabu Kommi <kommi.haribabu@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Fallout from PQhost() semantics changes
Date: Fri, 03 Aug 2018 11:21:38 -0400
Message-ID: <15907.1533309698@sss.pgh.pa.us> (raw)
In-Reply-To: <CAJrrPGfp_iT3Tn_h_aba060K89h_3RnmZchuTC6_92_fsvV1AQ@mail.gmail.com>
References: <23287.1533227021@sss.pgh.pa.us>
<CAJrrPGfp_iT3Tn_h_aba060K89h_3RnmZchuTC6_92_fsvV1AQ@mail.gmail.com>
Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> On Fri, Aug 3, 2018 at 2:24 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> What I think we should do about it is change those internal calls to
>> fetch connhost[].host directly instead of going through PQhost(), as
>> in the attached libpq-internal-PQhost-usage-1.patch. This will restore
>> the semantics to what they were pre-v10, including erroring out when
>> hostaddr is supplied without host.
> The Attached patch is good and I also verified that it is not missed anymore
> places that needs only a host.
Thanks for reviewing!
>> In fact, I think there's probably a good case for doing something
>> comparable to libpq-internal-PQhost-usage-1.patch all the way back.
>> In exactly what scenario is it sane to be comparing "/tmp" or
>> "localhost" to a server's SSL certificate?
> Yes, I agree that this problem present from a long, but may be till now
> everyone using along with host only?
Yeah, after thinking some more I'm not excited about changing this
pre-v10. There haven't been any field complaints, and there's also
a technical problem: the older versions didn't insert the default host
value (DEFAULT_PGSOCKET_DIR/DefaultHost) into the data structure but
just substituted it on-the-fly. So these call sites in fe-auth.c etc
would have to do that too, which seems messy and error-prone.
You could argue perhaps that we shouldn't be making that substitution
at all for fe-auth.c's purposes, but I think it's all right when both
host and hostaddr have been omitted. The default value then accurately
describes where we're connecting, and it's the same as the behavior
you'd get if you'd written out "host=/tmp" or "host=localhost" explicitly.
So I don't want to touch that behavior --- and indeed, I imagine that the
reason these call sites are using PQhost() is exactly that we wanted that
substitution to happen for these purposes.
Now that the default does get injected into the data structure, but only
when hostaddr isn't supplied, looking directly at the .host field does
exactly what we need in these places. But we'd need a bit of logic to get
comparable behavior pre-v10, and I don't think it's worth messing with.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, kommi.haribabu@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Fallout from PQhost() semantics changes
In-Reply-To: <15907.1533309698@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox