Refactoring of connection with password prompt loop for frontends
От | Michael Paquier |
---|---|
Тема | Refactoring of connection with password prompt loop for frontends |
Дата | |
Msg-id | 20190822074558.GG1683@paquier.xyz обсуждение исходный текст |
Ответы |
Re: Refactoring of connection with password prompt loop for frontends
|
Список | pgsql-hackers |
Hi all, In six places of the code tree (+ one in psql which is a bit different), we have the following pattern for frontend tools to connect to a backend with a password prompt, roughly like that: do { [...] conn = PQconnectdbParams(keywords, values, true); [...] if (PQstatus(conn) == CONNECTION_BAD && PQconnectionNeedsPassword(conn) && !have_password) { PQfinish(conn); simple_prompt("Password: ", password, sizeof(password), false); have_password = true; new_pass = true; } } while (new_pass); Attached is a tentative of patch to consolidate this logic across the tree. The patch is far from being in a committable state, and there are a couple of gotchas: - pg_dumpall merges connection string parameters, so it is much harder to plugin that the others, and I left it out on purpose. - Some code paths spawn a password prompt before the first connection attempt. For now I have left this first attempt out of the refactored logic, but I think that it is possible to consolidate that a bit more by enforcing a password prompt before doing the first connection attempt (somewhat related to the XXX portion in the patch). At the end it would be nice to not have to have a 100-byte-long field for the password buffer we have here and there. Unfortunately this comes with its limitations in pg_dump as savedPassword needs to be moved around and may be reused in the context. - I don't like the routine name connect_with_password_prompt() I introduced. Suggestions of better names are welcome :) - This also brings the point that some of our tools are not able to handle tri-values for passwords, so we may want to consolidate that as well. Among the positive points, this brings a lot of consolidation in terms of error handling, and this shaves a bit of code: 13 files changed, 190 insertions(+), 280 deletions(-) This moves the logic into src/fe_utils, which is natural as that's aimed only for frontends and because this also links to libpq. Please note that this links a bit with the refactoring of vacuumlo and oid2name logging I proposed a couple of days back (applying one patch or the other results in conflicts) because we need to have frontends initialized for logging in order to be able to log errors in the refactored routine: https://www.postgresql.org/message-id/20190820012819.GA8326@paquier.xyz This one should be merged first IMO, but that's a story for the other thread. This compiles, and passes all regression tests so it is possible to play with it easily, still it needs much more testing and love. Any thoughts? I am adding that to the next commit fest. Thanks, -- Michael
Вложения
В списке pgsql-hackers по дате отправления: