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
