From: | Greg Nancarrow <gregn4422(at)gmail(dot)com> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Cc: | Andrew Dunstan <adunstan(at)postgresql(dot)org> |
Subject: | Re: libpq sslpassword parameter and callback function |
Date: | 2019-11-29 03:25:45 |
Message-ID: | 157499794571.13092.9894985502685806190.pgcf@coridan.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, failed
Hi Andrew,
I've reviewed your "libpq sslpassword parameter and callback function" patch (0001-libpq-sslpassword-der-support.patch), and only found a few minor things (otherwise it looked good to me):
1) There's a few trailing white-space warnings on patch application (from where you modified to skip 2 of the tests):
git apply 0001-libpq-sslpassword-der-support.patch
0001-libpq-sslpassword-der-support.patch:649: trailing whitespace.
# so they don't hang. For now they are not performed.
0001-libpq-sslpassword-der-support.patch:659: trailing whitespace.
warning: 2 lines add whitespace errors.
2) src/interfaces/libpq/libpq-fe.h
The following portion of the comment should be removed.
+ * 2ndQPostgres extension. If you need to be compatible with unpatched libpq
+ * you must dlsym() these.
3) Documentation for the "PQsslpassword" function should be added to the libpq "33.2 Connection Status Functions" section.
I made the following notes about how/what I reviewed and tested:
- Applied patch and built Postgres (--with-openssl --enable-tap-tests), checked build output
- Checked patch code modifications (format, logic, memory usage, efficiency, corner cases etc.)
- Built documentation and checked updated portions (format, grammar, details, completeness etc.)
- Checked test updates
- Ran updated contrib/dblink tests - confirmed all passed
- Ran updated SSL (TAP) tests - confirmed all passed
- Ran "make installcheck-world", as per review requirements
- Wrote small libpq-based app to test:
- new APIs (PQsslpassword, PQsetSSLKeyPassHook, PQgetSSLKeyPassHook, PQdefaultSSLKeyPassHook)
- passphrase-protected key with/without patch
- patch with/without new key password callack
- patch and certificate with/without pass phrase protection on key
- default callback, callback delegation
- PEM/DER keys
Regards,
Greg
From | Date | Subject | |
---|---|---|---|
Next Message | Etsuro Fujita | 2019-11-29 03:35:33 | Re: A problem about partitionwise join |
Previous Message | Richard Guo | 2019-11-29 03:07:53 | Re: A problem about partitionwise join |