Re: Broken SSL tests in master

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Andreas Karlsson <andreas(at)proxel(dot)se>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, mithun(dot)cy(at)enterprisedb(dot)com, tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com
Subject: Re: Broken SSL tests in master
Date: 2016-11-25 03:54:37
Message-ID: CAB7nPqTti-eP42XOTSnZHj5Ow5HPJztzKK47jbQz35Qyu_jttg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 25, 2016 at 8:15 AM, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
> On 11/24/2016 10:38 PM, Andreas Karlsson wrote:
>> To me it feels like the proper fix would be to make PQHost() return the
>> value of the host parameter rather than the hostaddr (maybe add a new
>> field in the pg_conn_host struct). But would be a behaviour change which
>> might break someones application. Thoughts?

Thanks for digging up the root cause. I can see the problem with HEAD as well.

> I have attached a proof of concept patch for this. Remaining work is
> investigating all the callers of PQhost() and see if any of them are
> negatively affected by this patch and cleaning up the code some.

This needs some thoughts, but first I need to understand the
whereabouts of this refactoring work in 9a1d0af4 as well as the
structures that 274bb2b3 has introduced. From what I can see you are
duplicating some logic parsing the pghost string when there is a
pghostaddr set, so my first guess is that you are breaking some of the
intentions behind this code by patching it this way... I am adding in
CC Robert, Mithun and Tsunakawa-san who worked on that. On my side,
I'll need some time to study and understand this new code, that's
necessary before looking at your patch in details, there could be a
more elegant solution. And we had better address this issue before
looking more into the SSL reload patch.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2016-11-25 04:38:45 Re: [sqlsmith] Failed assertion in parallel worker in ExecInitSubPlan
Previous Message Alvaro Herrera 2016-11-25 03:11:58 Re: Declarative partitioning - another take