> On 17 Mar 2022, at 09:05, Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > At Thu, 17 Mar 2022 16:22:14 +0900, Michael Paquier <mich...@paquier.xyz> > wrote in >> On Thu, Mar 17, 2022 at 02:59:26PM +0900, Michael Paquier wrote: >>> In both cases, enforcing sslcrl to a value of "invalid" interferes >>> with the failure scenario we expect from sslcrldir. It is possible to >>> bypass that with something like the attached, but that's a kind of >>> ugly hack. Another alternative would be to drop those two tests, and >>> I am not sure how much we care about these two negative scenarios.
I really don't think we should drop tests based on these premises, at least not until it's raised as a problem/inconvenience but more hackers. I would prefer to instead extend the error message with hints that ~/.postgresql contents could've affected test outcome. But, as the v2 patch handles it this is mostly academic at this point. >> Actually, there is a trick I have recalled here: we can enforce sslcrl >> to an empty value in the connection string after the default. This >> still ensures that the test won't pick up any SSL data from the local >> environment and avoids any interferences of OpenSSL's >> X509_STORE_load_locations(). This gives a much simpler and cleaner >> patch. > Ah! I didn't have a thought that we can specify the same parameter > twice. It looks like clean and robust. $default_ssl_connstr contains > all required options in PQconninfoOptions[]. +1 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. -- Daniel Gustafsson https://vmware.com/
