From 87eeadaea1439693f6108ff599d750842ca6f338 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Thu, 19 Sep 2024 15:39:01 +0900 Subject: [PATCH] psql: Clean up more aggressively state of \bind[_named], \parse and \close This fixes a couple of issues with the psql meta-commands mentioned above when called repeatedly: - The statement name is reset for each call. If a command errors out, its send_mode would still be set, causing an incorrect path to be taken when processing a query. For \bind_named, this could trigger an assertion failure as a statement name is always expected for this meta-command. This issue has been introduced by d55322b0da60. - The memory allocated for bind parameters can be leaked. This is a bug enlarged by d55322b0da60 that exists since 5b66de3433e2, as it is also possible to leak memory with \bind in v16 and v17. This requires a fix that will be done on the affected branches separately. This issue is taken care of here for HEAD. This patch tightens the cleanup of the state used for the extended protocol meta-commands (bind parameters, send mode, statement name) by doing it before running each meta-command on top of doing it once a query has been processed, avoiding any leaks and the inconsistencies when mixing calls, by refactoring the cleanup in a single routine used in all the code paths where this step is required. Reported-by: Alexander Lakhin Author: Anthonin Bonnefoy Discussion: https://postgr.es/m/2e5b89af-a351-ff0a-000c-037ac28314ab@gmail.com --- src/bin/psql/command.c | 12 +++--- src/bin/psql/common.c | 59 +++++++++++++++++++----------- src/bin/psql/common.h | 1 + src/test/regress/expected/psql.out | 9 +++++ src/test/regress/sql/psql.sql | 5 +++ 5 files changed, 59 insertions(+), 27 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 4dfc7b2d857..2bb87897505 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -483,8 +483,7 @@ exec_command_bind(PsqlScanState scan_state, bool active_branch) int nparams = 0; int nalloc = 0; - pset.bind_params = NULL; - pset.stmtName = NULL; + clean_extended_state(); while ((opt = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, false))) { @@ -521,8 +520,7 @@ exec_command_bind_named(PsqlScanState scan_state, bool active_branch, int nparams = 0; int nalloc = 0; - pset.bind_params = NULL; - pset.stmtName = NULL; + clean_extended_state(); /* get the mandatory prepared statement name */ opt = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, false); @@ -719,7 +717,8 @@ exec_command_close(PsqlScanState scan_state, bool active_branch, const char *cmd char *opt = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, false); - pset.stmtName = NULL; + clean_extended_state(); + if (!opt) { pg_log_error("\\%s: missing required argument", cmd); @@ -2205,7 +2204,8 @@ exec_command_parse(PsqlScanState scan_state, bool active_branch, char *opt = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, false); - pset.stmtName = NULL; + clean_extended_state(); + if (!opt) { pg_log_error("\\%s: missing required argument", cmd); diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 066dccbd841..8a9211db41a 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -1275,27 +1275,7 @@ sendquery_cleanup: } /* clean up after extended protocol queries */ - switch (pset.send_mode) - { - case PSQL_SEND_EXTENDED_CLOSE: /* \close */ - free(pset.stmtName); - break; - case PSQL_SEND_EXTENDED_PARSE: /* \parse */ - free(pset.stmtName); - break; - case PSQL_SEND_EXTENDED_QUERY_PARAMS: /* \bind */ - case PSQL_SEND_EXTENDED_QUERY_PREPARED: /* \bind_named */ - for (i = 0; i < pset.bind_nparams; i++) - free(pset.bind_params[i]); - free(pset.bind_params); - free(pset.stmtName); - pset.bind_params = NULL; - break; - case PSQL_SEND_QUERY: - break; - } - pset.stmtName = NULL; - pset.send_mode = PSQL_SEND_QUERY; + clean_extended_state(); /* reset \gset trigger */ if (pset.gset_prefix) @@ -2287,6 +2267,43 @@ uri_prefix_length(const char *connstr) return 0; } +/* + * Reset state related to extended query protocol + * + * Clean up any state related to bind parameters, statement name and + * PSQL_SEND_MODE. This needs to be called after processing a query or when + * running a new meta-command that uses the extended query protocol, like + * \parse, \bind, etc. + */ +void +clean_extended_state(void) +{ + int i; + + switch (pset.send_mode) + { + case PSQL_SEND_EXTENDED_CLOSE: /* \close */ + free(pset.stmtName); + break; + case PSQL_SEND_EXTENDED_PARSE: /* \parse */ + free(pset.stmtName); + break; + case PSQL_SEND_EXTENDED_QUERY_PARAMS: /* \bind */ + case PSQL_SEND_EXTENDED_QUERY_PREPARED: /* \bind_named */ + for (i = 0; i < pset.bind_nparams; i++) + free(pset.bind_params[i]); + free(pset.bind_params); + free(pset.stmtName); + pset.bind_params = NULL; + break; + case PSQL_SEND_QUERY: + break; + } + + pset.stmtName = NULL; + pset.send_mode = PSQL_SEND_QUERY; +} + /* * Recognized connection string either starts with a valid URI prefix or * contains a "=" in it. diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h index 6efe12274fe..e3762a2c6c7 100644 --- a/src/bin/psql/common.h +++ b/src/bin/psql/common.h @@ -41,6 +41,7 @@ extern bool standard_strings(void); extern const char *session_username(void); extern void expand_tilde(char **filename); +extern void clean_extended_state(void); extern bool recognized_connection_string(const char *connstr); diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index 6aeb7cb9636..cf040fbd803 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -132,6 +132,15 @@ SELECT $1, $2 \parse stmt3 foo | bar (1 row) +-- Repeated calls. The second call generates an error, cleaning up the +-- statement name set by the first call. +\bind_named stmt4 +\bind_named +\bind_named: missing required argument +\g +ERROR: there is no parameter $1 +LINE 1: SELECT $1, $2 + ^ -- \close (extended query protocol) \close \close: missing required argument diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql index 0a2f8b46922..8de90c805c0 100644 --- a/src/test/regress/sql/psql.sql +++ b/src/test/regress/sql/psql.sql @@ -58,6 +58,11 @@ SELECT $1, $2 \parse stmt3 \bind_named stmt1 \g \bind_named stmt2 'foo' \g \bind_named stmt3 'foo' 'bar' \g +-- Repeated calls. The second call generates an error, cleaning up the +-- statement name set by the first call. +\bind_named stmt4 +\bind_named +\g -- \close (extended query protocol) \close -- 2.39.5