Skip to content

Commit 296ad2b

Browse files
committed
Fix GH-21869: pdo_pgsql DEALLOCATE in destructor can poison the enclosing transaction.
On libpq < 17 the raw DEALLOCATE can fail (e.g. Aurora DSQL rejects it), which aborts the user's transaction and turns the next COMMIT into a silent ROLLBACK. Wrap it in a SAVEPOINT so a failure is contained, and skip it when the transaction is already aborted or the connection is gone.
1 parent 7bd27e7 commit 296ad2b

2 files changed

Lines changed: 83 additions & 3 deletions

File tree

ext/pdo_pgsql/pgsql_statement.c

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,43 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt)
7979
// TODO (??) libpq does not support close statement protocol < postgres 17
8080
// check if we can circumvent this.
8181
char *q = NULL;
82-
spprintf(&q, 0, "DEALLOCATE %s", S->stmt_name);
83-
res = PQexec(H->server, q);
84-
efree(q);
82+
PGTransactionStatusType tstatus = PQtransactionStatus(H->server);
83+
84+
switch (tstatus) {
85+
case PQTRANS_ACTIVE:
86+
case PQTRANS_INERROR:
87+
case PQTRANS_UNKNOWN:
88+
res = NULL;
89+
break;
90+
case PQTRANS_INTRANS:
91+
spprintf(&q, 0,
92+
"SAVEPOINT pdo_pgsql_deallocate_%s;"
93+
"DEALLOCATE %s;"
94+
"RELEASE SAVEPOINT pdo_pgsql_deallocate_%s",
95+
S->stmt_name,
96+
S->stmt_name,
97+
S->stmt_name);
98+
res = PQexec(H->server, q);
99+
efree(q);
100+
if (res && PQresultStatus(res) != PGRES_COMMAND_OK) {
101+
spprintf(&q, 0,
102+
"ROLLBACK TO SAVEPOINT pdo_pgsql_deallocate_%s;"
103+
"RELEASE SAVEPOINT pdo_pgsql_deallocate_%s",
104+
S->stmt_name,
105+
S->stmt_name);
106+
PGresult *rollres;
107+
PQclear(res);
108+
rollres = PQexec(H->server, q);
109+
res = rollres;
110+
efree(q);
111+
}
112+
break;
113+
default:
114+
spprintf(&q, 0, "DEALLOCATE %s", S->stmt_name);
115+
res = PQexec(H->server, q);
116+
efree(q);
117+
break;
118+
}
85119
#else
86120
res = PQclosePrepared(H->server, S->stmt_name);
87121
#endif

ext/pdo_pgsql/tests/gh21869.phpt

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
GH-21869 pdo_pgsql: DEALLOCATE failure in destructor must not poison the enclosing transaction
3+
--EXTENSIONS--
4+
pdo
5+
pdo_pgsql
6+
--SKIPIF--
7+
<?php
8+
require __DIR__ . '/config.inc';
9+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
10+
PDOTest::skip();
11+
$pdo = PDOTest::factory();
12+
if (version_compare($pdo->getAttribute(PDO::ATTR_CLIENT_VERSION), '17', '>=')) {
13+
die('skip libpq >= 17 uses PQclosePrepared instead of the DEALLOCATE query');
14+
}
15+
?>
16+
--FILE--
17+
<?php
18+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
19+
$pdo = PDOTest::test_factory(__DIR__ . '/common.phpt');
20+
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
21+
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
22+
23+
$pdo->exec('CREATE TABLE gh21869 (a integer not null)');
24+
25+
$pdo->beginTransaction();
26+
$stmt = $pdo->prepare('INSERT INTO gh21869 (a) VALUES (?)');
27+
$stmt->execute([1]);
28+
29+
foreach ($pdo->query('SELECT name FROM pg_prepared_statements') as $row) {
30+
$pdo->exec('DEALLOCATE ' . $row['name']);
31+
}
32+
33+
unset($stmt);
34+
35+
$pdo->commit();
36+
37+
var_dump((int) $pdo->query('SELECT count(*) FROM gh21869')->fetchColumn());
38+
?>
39+
--CLEAN--
40+
<?php
41+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
42+
$pdo = PDOTest::test_factory(__DIR__ . '/common.phpt');
43+
$pdo->exec('DROP TABLE IF EXISTS gh21869');
44+
?>
45+
--EXPECT--
46+
int(1)

0 commit comments

Comments
 (0)