Skip to content

Commit 87eeada

Browse files
michaelpqmichaelpq
committed
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 d55322b. - The memory allocated for bind parameters can be leaked. This is a bug enlarged by d55322b that exists since 5b66de3, 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
1 parent d69a3f4 commit 87eeada

File tree

5 files changed

+59
-27
lines changed

5 files changed

+59
-27
lines changed

‎src/bin/psql/command.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,7 @@ exec_command_bind(PsqlScanState scan_state, bool active_branch)
483483
int nparams = 0;
484484
int nalloc = 0;
485485

486-
pset.bind_params = NULL;
487-
pset.stmtName = NULL;
486+
clean_extended_state();
488487

489488
while ((opt = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, false)))
490489
{
@@ -521,8 +520,7 @@ exec_command_bind_named(PsqlScanState scan_state, bool active_branch,
521520
int nparams = 0;
522521
int nalloc = 0;
523522

524-
pset.bind_params = NULL;
525-
pset.stmtName = NULL;
523+
clean_extended_state();
526524

527525
/* get the mandatory prepared statement name */
528526
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
719717
char *opt = psql_scan_slash_option(scan_state,
720718
OT_NORMAL, NULL, false);
721719

722-
pset.stmtName = NULL;
720+
clean_extended_state();
721+
723722
if (!opt)
724723
{
725724
pg_log_error("\\%s: missing required argument", cmd);
@@ -2205,7 +2204,8 @@ exec_command_parse(PsqlScanState scan_state, bool active_branch,
22052204
char *opt = psql_scan_slash_option(scan_state,
22062205
OT_NORMAL, NULL, false);
22072206

2208-
pset.stmtName = NULL;
2207+
clean_extended_state();
2208+
22092209
if (!opt)
22102210
{
22112211
pg_log_error("\\%s: missing required argument", cmd);

‎src/bin/psql/common.c

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,27 +1275,7 @@ SendQuery(const char *query)
12751275
}
12761276

12771277
/* clean up after extended protocol queries */
1278-
switch (pset.send_mode)
1279-
{
1280-
case PSQL_SEND_EXTENDED_CLOSE: /* \close */
1281-
free(pset.stmtName);
1282-
break;
1283-
case PSQL_SEND_EXTENDED_PARSE: /* \parse */
1284-
free(pset.stmtName);
1285-
break;
1286-
case PSQL_SEND_EXTENDED_QUERY_PARAMS: /* \bind */
1287-
case PSQL_SEND_EXTENDED_QUERY_PREPARED: /* \bind_named */
1288-
for (i = 0; i < pset.bind_nparams; i++)
1289-
free(pset.bind_params[i]);
1290-
free(pset.bind_params);
1291-
free(pset.stmtName);
1292-
pset.bind_params = NULL;
1293-
break;
1294-
case PSQL_SEND_QUERY:
1295-
break;
1296-
}
1297-
pset.stmtName = NULL;
1298-
pset.send_mode = PSQL_SEND_QUERY;
1278+
clean_extended_state();
12991279

13001280
/* reset \gset trigger */
13011281
if (pset.gset_prefix)
@@ -2287,6 +2267,43 @@ uri_prefix_length(const char *connstr)
22872267
return 0;
22882268
}
22892269

2270+
/*
2271+
* Reset state related to extended query protocol
2272+
*
2273+
* Clean up any state related to bind parameters, statement name and
2274+
* PSQL_SEND_MODE. This needs to be called after processing a query or when
2275+
* running a new meta-command that uses the extended query protocol, like
2276+
* \parse, \bind, etc.
2277+
*/
2278+
void
2279+
clean_extended_state(void)
2280+
{
2281+
int i;
2282+
2283+
switch (pset.send_mode)
2284+
{
2285+
case PSQL_SEND_EXTENDED_CLOSE: /* \close */
2286+
free(pset.stmtName);
2287+
break;
2288+
case PSQL_SEND_EXTENDED_PARSE: /* \parse */
2289+
free(pset.stmtName);
2290+
break;
2291+
case PSQL_SEND_EXTENDED_QUERY_PARAMS: /* \bind */
2292+
case PSQL_SEND_EXTENDED_QUERY_PREPARED: /* \bind_named */
2293+
for (i = 0; i < pset.bind_nparams; i++)
2294+
free(pset.bind_params[i]);
2295+
free(pset.bind_params);
2296+
free(pset.stmtName);
2297+
pset.bind_params = NULL;
2298+
break;
2299+
case PSQL_SEND_QUERY:
2300+
break;
2301+
}
2302+
2303+
pset.stmtName = NULL;
2304+
pset.send_mode = PSQL_SEND_QUERY;
2305+
}
2306+
22902307
/*
22912308
* Recognized connection string either starts with a valid URI prefix or
22922309
* contains a "=" in it.

‎src/bin/psql/common.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ extern bool standard_strings(void);
4141
extern const char *session_username(void);
4242

4343
extern void expand_tilde(char **filename);
44+
extern void clean_extended_state(void);
4445

4546
extern bool recognized_connection_string(const char *connstr);
4647

‎src/test/regress/expected/psql.out

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,15 @@ SELECT $1, $2 \parse stmt3
132132
foo | bar
133133
(1 row)
134134

135+
-- Repeated calls. The second call generates an error, cleaning up the
136+
-- statement name set by the first call.
137+
\bind_named stmt4
138+
\bind_named
139+
\bind_named: missing required argument
140+
\g
141+
ERROR: there is no parameter $1
142+
LINE 1: SELECT $1, $2
143+
^
135144
-- \close (extended query protocol)
136145
\close
137146
\close: missing required argument

‎src/test/regress/sql/psql.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ SELECT $1, $2 \parse stmt3
5858
\bind_named stmt1 \g
5959
\bind_named stmt2 'foo' \g
6060
\bind_named stmt3 'foo' 'bar' \g
61+
-- Repeated calls. The second call generates an error, cleaning up the
62+
-- statement name set by the first call.
63+
\bind_named stmt4
64+
\bind_named
65+
\g
6166

6267
-- \close (extended query protocol)
6368
\close

0 commit comments

Comments
 (0)