On 3/17/22 21:02, Michael Paquier wrote:
> On Thu, Mar 17, 2022 at 02:28:49PM +0100, Daniel Gustafsson wrote:
>> One small concern though. This hunk:
>>
>> +my $default_ssl_connstr = "sslkey=invalid sslcert=invalid sslrootcert=invalid sslcrl=invalid sslcrldir=invalid";
>> +
>> $common_connstr =
>> - "user=ssltestuser dbname=trustdb sslcert=invalid hostaddr=$SERVERHOSTADDR host=common-name.pg-ssltest.test";
>> + "$default_ssl_connstr user=ssltestuser dbname=trustdb hostaddr=$SERVERHOSTADDR
host=common-name.pg-ssltest.test";
>>
>> ..together with the following changes along the lines of:
>>
>> - "$common_connstr sslrootcert=invalid sslmode=require",
>> + "$common_connstr sslmode=require",
>>
>> ..is making it fairly hard to read the test and visualize what the connection
>> string is and how the test should behave. I don't have a better idea off the
>> top of my head right now, but I think this is an area to revisit and improve
>> on.
> I agree that this makes this set of three tests harder to follow, as
> we expect a root cert to *not* be set locally. Keeping the behavior
> documented in each individual string would be better, even if that
> duplicates more the keys in those final strings.
>
> Another thing that Horiguchi-san has pointed out upthread (?) is 003,
> where it is also possible to trigger failures once the environment is
> hijacked. The attached allows the full test suite to pass without
> issues on my side.
LGTM
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
