mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Fix crash when describing a FETCH statement whose cursor is gone
FetchStatementTargetList() dereferenced the portal named by a FetchStmt without checking that it still exists. With the extended query protocol, a client can sent a Parse "FETCH n FROM c" while cursor "c" is open (which caches a result's TupleDesc), issue a CLOSE on cursor "c", and then Describe the prepared statement or a portal bound to it. Describe would re-resolve the target list through the named cursor, and crash. In a non-assert builds, this causes a NULL pointer dereference. An assertion was triggered in assert builds slightly before the pointer dereference. This commit changes the describe of a FetchStmt to match with the ExecuteStmt case: if the named cursor's portal is invalid, fail rather than assume that the portal should always exist. A test case is added to libpq_pipeline, backpatchable all the way down. Author: Dirkjan Bussink <d.bussink@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/7DE248EB-7509-4F8A-9F37-4694A40BBAB7@gmail.com Backpatch-through: 14
This commit is contained in:
@@ -389,7 +389,11 @@ FetchStatementTargetList(Node *stmt)
|
||||
|
||||
Assert(!fstmt->ismove);
|
||||
subportal = GetPortalByName(fstmt->portalname);
|
||||
Assert(PortalIsValid(subportal));
|
||||
if (!PortalIsValid(subportal))
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_UNDEFINED_CURSOR),
|
||||
errmsg("cursor \"%s\" does not exist",
|
||||
fstmt->portalname)));
|
||||
return FetchPortalTargetList(subportal);
|
||||
}
|
||||
if (IsA(stmt, ExecuteStmt))
|
||||
|
||||
@@ -456,6 +456,52 @@ test_cancel(PGconn *conn)
|
||||
fprintf(stderr, "ok\n");
|
||||
}
|
||||
|
||||
/*
|
||||
* Test Describe of a prepared FETCH statement after the cursor it
|
||||
* references has been closed.
|
||||
*/
|
||||
static void
|
||||
test_describe_fetch(PGconn *conn)
|
||||
{
|
||||
PGresult *res;
|
||||
|
||||
fprintf(stderr, "test cursor describe and fetch... ");
|
||||
|
||||
res = PQexec(conn, "BEGIN");
|
||||
if (PQresultStatus(res) != PGRES_COMMAND_OK)
|
||||
pg_fatal("BEGIN failed: %s", PQerrorMessage(conn));
|
||||
PQclear(res);
|
||||
|
||||
res = PQexec(conn, "DECLARE fetch_cursor CURSOR FOR SELECT 1");
|
||||
if (PQresultStatus(res) != PGRES_COMMAND_OK)
|
||||
pg_fatal("DECLARE CURSOR failed: %s", PQerrorMessage(conn));
|
||||
PQclear(res);
|
||||
|
||||
/* prepare while the cursor exists, so that it caches a result desc */
|
||||
res = PQprepare(conn, "fetch_one", "FETCH 1 FROM fetch_cursor", 0, NULL);
|
||||
if (PQresultStatus(res) != PGRES_COMMAND_OK)
|
||||
pg_fatal("PQprepare failed: %s", PQerrorMessage(conn));
|
||||
PQclear(res);
|
||||
|
||||
res = PQexec(conn, "CLOSE fetch_cursor");
|
||||
if (PQresultStatus(res) != PGRES_COMMAND_OK)
|
||||
pg_fatal("CLOSE failed: %s", PQerrorMessage(conn));
|
||||
PQclear(res);
|
||||
|
||||
/* describe fails after the cursor has been closed */
|
||||
res = PQdescribePrepared(conn, "fetch_one");
|
||||
if (PQresultStatus(res) != PGRES_FATAL_ERROR)
|
||||
pg_fatal("expected FATAL_ERROR, got %s", PQresStatus(PQresultStatus(res)));
|
||||
PQclear(res);
|
||||
|
||||
res = PQexec(conn, "ROLLBACK");
|
||||
if (PQresultStatus(res) != PGRES_COMMAND_OK)
|
||||
pg_fatal("ROLLBACK failed: %s", PQerrorMessage(conn));
|
||||
PQclear(res);
|
||||
|
||||
fprintf(stderr, "ok\n");
|
||||
}
|
||||
|
||||
static void
|
||||
test_disallowed_in_pipeline(PGconn *conn)
|
||||
{
|
||||
@@ -2117,6 +2163,7 @@ static void
|
||||
print_test_list(void)
|
||||
{
|
||||
printf("cancel\n");
|
||||
printf("describe_fetch\n");
|
||||
printf("disallowed_in_pipeline\n");
|
||||
printf("multi_pipelines\n");
|
||||
printf("nosync\n");
|
||||
@@ -2223,6 +2270,8 @@ main(int argc, char **argv)
|
||||
|
||||
if (strcmp(testname, "cancel") == 0)
|
||||
test_cancel(conn);
|
||||
else if (strcmp(testname, "describe_fetch") == 0)
|
||||
test_describe_fetch(conn);
|
||||
else if (strcmp(testname, "disallowed_in_pipeline") == 0)
|
||||
test_disallowed_in_pipeline(conn);
|
||||
else if (strcmp(testname, "multi_pipelines") == 0)
|
||||
|
||||
Reference in New Issue
Block a user