Skip to content

Commit 810a9e5

Browse files
devnexendevnexen
committed
Merge branch 'PHP-8.5'
* PHP-8.5: ext/pdo_pgsql: Fix CURSOR_SCROLL statements closing a nonexistent cursor # Conflicts: # ext/pdo_pgsql/config.m4 # ext/pdo_pgsql/pgsql_statement.c
2 parents 67135cb + f3607f1 commit 810a9e5

10 files changed

Lines changed: 298 additions & 9 deletions

‎ext/pdo_pgsql/config.m4‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@ if test "$PHP_PDO_PGSQL" != "no"; then
2525
or later).])],,
2626
[$PGSQL_LIBS])
2727

28+
PHP_CHECK_LIBRARY([pq], [PQclosePortal],
29+
[AC_DEFINE([HAVE_PQCLOSEPORTAL], [1],
30+
[Define to 1 if libpq has the 'PQclosePortal' function (PostgreSQL 17
31+
or later).])],,
32+
[$PGSQL_LIBS])
33+
2834
old_CFLAGS=$CFLAGS
2935
CFLAGS="$CFLAGS $PGSQL_CFLAGS"
3036

‎ext/pdo_pgsql/config.w32‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ if (PHP_PDO_PGSQL != "no") {
1010
AC_DEFINE('HAVE_PG_RESULT_MEMORY_SIZE', 1, "Define to 1 if libpq has the 'PQresultMemorySize' function (PostgreSQL 12 or later).");
1111
AC_DEFINE('HAVE_PDO_PGSQL', 1, "Define to 1 if the PHP extension 'pdo_pgsql' is available.");
1212

13+
if (GREP_HEADER("libpq-fe.h", "PQclosePortal", PHP_PDO_PGSQL + "\\include;" + PHP_PHP_BUILD + "\\include\\pgsql;" + PHP_PHP_BUILD + "\\include\\libpq;")) {
14+
AC_DEFINE('HAVE_PQCLOSEPORTAL', 1, "Define to 1 if libpq has the 'PQclosePortal' function (PostgreSQL 17 or later).");
15+
}
16+
1317
ADD_EXTENSION_DEP('pdo_pgsql', 'pdo');
1418
ADD_MAKEFILE_FRAGMENT();
1519
} else {

‎ext/pdo_pgsql/pgsql_statement.c‎

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,47 @@ static bool pgsql_result_status_ok(ExecStatusType status)
7373
}
7474
}
7575

76+
#ifndef HAVE_PQCLOSEPORTAL
77+
static bool pdo_pgsql_try_cmd(const char *cmd, const char *ok_sqlstate, pdo_pgsql_db_handle *H)
78+
{
79+
bool result = false;
80+
char *q = NULL;
81+
PGresult *res = NULL;
82+
83+
PGTransactionStatusType status = PQtransactionStatus(H->server);
84+
85+
switch (status) {
86+
case PQTRANS_ACTIVE:
87+
case PQTRANS_INERROR:
88+
break;
89+
case PQTRANS_INTRANS: /* failure must not abort the caller's transaction */
90+
/* PQexec does not run the statements following a failed one */
91+
spprintf(&q, 0, "SAVEPOINT pdo_pgsql_savepoint; %s; RELEASE SAVEPOINT pdo_pgsql_savepoint;", cmd);
92+
res = PQexec(H->server, q);
93+
94+
if (PQresultStatus(res) != PGRES_COMMAND_OK) {
95+
PQclear(PQexec(H->server, "ROLLBACK TO SAVEPOINT pdo_pgsql_savepoint; RELEASE SAVEPOINT pdo_pgsql_savepoint"));
96+
}
97+
98+
break;
99+
default:
100+
res = PQexec(H->server, cmd);
101+
}
102+
103+
if (PQresultStatus(res) == PGRES_COMMAND_OK) {
104+
result = true;
105+
} else if (res) {
106+
const char *sqlstate = pdo_pgsql_sqlstate(res);
107+
108+
result = sqlstate && !strcmp(sqlstate, ok_sqlstate);
109+
}
110+
111+
if (q) efree(q);
112+
if (res) PQclear(res);
113+
114+
return result;
115+
}
116+
#endif
76117

77118

78119
static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
@@ -193,15 +234,16 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt)
193234
}
194235

195236
if (S->cursor_name) {
196-
if (server_obj_usable) {
237+
if (S->is_cursor_declared && server_obj_usable) {
197238
pdo_pgsql_db_handle *H = S->H;
198-
char *q = NULL;
199-
PGresult *res;
200-
239+
#ifndef HAVE_PQCLOSEPORTAL
240+
char *q;
201241
spprintf(&q, 0, "CLOSE %s", S->cursor_name);
202-
res = PQexec(H->server, q);
242+
pdo_pgsql_try_cmd(q, "34000", H); /* 34000: invalid_cursor_name */
203243
efree(q);
204-
if (res) PQclear(res);
244+
#else
245+
PQclear(PQclosePortal(H->server, S->cursor_name));
246+
#endif
205247
}
206248
efree(S->cursor_name);
207249
S->cursor_name = NULL;
@@ -241,10 +283,25 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt)
241283
if (S->cursor_name) {
242284
char *q = NULL;
243285

244-
if (S->is_prepared) {
286+
if (S->is_cursor_declared) {
287+
#ifndef HAVE_PQCLOSEPORTAL
245288
spprintf(&q, 0, "CLOSE %s", S->cursor_name);
246-
PQclear(PQexec(H->server, q));
289+
290+
/* 34000: invalid_cursor_name */
291+
if (pdo_pgsql_try_cmd(q, "34000", H)) {
292+
S->is_cursor_declared = false;
293+
}
294+
247295
efree(q);
296+
#else
297+
PGresult *res = PQclosePortal(H->server, S->cursor_name);
298+
299+
if (PQresultStatus(res) == PGRES_COMMAND_OK) {
300+
S->is_cursor_declared = false;
301+
}
302+
303+
PQclear(res);
304+
#endif
248305
}
249306

250307
spprintf(&q, 0, "DECLARE %s SCROLL CURSOR WITH HOLD FOR %s", S->cursor_name, ZSTR_VAL(stmt->active_query_string));
@@ -260,7 +317,7 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt)
260317
PQclear(S->result);
261318

262319
/* the cursor was declared correctly */
263-
S->is_prepared = true;
320+
S->is_cursor_declared = true;
264321

265322
/* fetch to be able to get the number of tuples later, but don't advance the cursor pointer */
266323
spprintf(&q, 0, "FETCH FORWARD 0 FROM %s", S->cursor_name);

‎ext/pdo_pgsql/php_pdo_pgsql_int.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ struct pdo_pgsql_stmt {
6969
int current_row;
7070
zend_long chunk_size;
7171
bool is_prepared;
72+
bool is_cursor_declared;
7273
bool is_unbuffered;
7374
bool is_running_unbuffered;
7475
};

‎ext/pdo_pgsql/tests/cursor_scroll_close_failed_before_redeclare.phpt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
PDO PgSQL PDO::CURSOR_SCROLL keeps track of a held cursor when the CLOSE before a re-declare fails
3+
--EXTENSIONS--
4+
pdo_pgsql
5+
--SKIPIF--
6+
<?php
7+
require __DIR__ . '/config.inc';
8+
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
9+
PDOTest::skip();
10+
?>
11+
--FILE--
12+
<?php
13+
14+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
15+
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
16+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
17+
18+
$stmt = $db->prepare('SELECT CAST(:v AS int)', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
19+
$stmt->execute([':v' => '1']);
20+
21+
$db->beginTransaction();
22+
23+
try {
24+
$db->exec('SELECT 1 / 0');
25+
} catch (PDOException $e) {
26+
echo $e::class, ': ', $e->getCode(), PHP_EOL;
27+
}
28+
29+
try {
30+
$stmt->execute([':v' => '2']);
31+
} catch (PDOException $e) {
32+
echo $e::class, ': ', $e->getCode(), PHP_EOL;
33+
}
34+
35+
$db->rollBack();
36+
unset($stmt);
37+
38+
var_dump($db->query("SELECT count(*) FROM pg_cursors WHERE name LIKE 'pdo\_crsr\_%'")->fetchColumn());
39+
40+
?>
41+
--EXPECT--
42+
PDOException: 22012
43+
PDOException: 25P02
44+
string(1) "0"

‎ext/pdo_pgsql/tests/cursor_scroll_discard_all.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
PDO PgSQL PDO::CURSOR_SCROLL cursor destroyed by DISCARD ALL does not break the transaction
3+
--EXTENSIONS--
4+
pdo_pgsql
5+
--SKIPIF--
6+
<?php
7+
require __DIR__ . '/config.inc';
8+
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
9+
PDOTest::skip();
10+
?>
11+
--FILE--
12+
<?php
13+
14+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
15+
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
16+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
17+
18+
$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
19+
$stmt->execute();
20+
21+
/* a connection pooler issues this when handing the connection back */
22+
$db->exec('DISCARD ALL');
23+
24+
$db->beginTransaction();
25+
26+
unset($stmt);
27+
28+
echo $db->query('SELECT 2')->fetchColumn(), PHP_EOL;
29+
30+
$db->rollBack();
31+
32+
echo 'Done', PHP_EOL;
33+
34+
?>
35+
--EXPECT--
36+
2
37+
Done

‎ext/pdo_pgsql/tests/cursor_scroll_failed_redeclare.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE after a failed re-declare
3+
--EXTENSIONS--
4+
pdo_pgsql
5+
--SKIPIF--
6+
<?php
7+
require __DIR__ . '/config.inc';
8+
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
9+
PDOTest::skip();
10+
?>
11+
--FILE--
12+
<?php
13+
14+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
15+
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
16+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
17+
18+
$stmt = $db->prepare('SELECT CAST(:v AS int)', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
19+
$stmt->execute([':v' => '1']);
20+
21+
try {
22+
$stmt->execute([':v' => 'not an int']);
23+
} catch (PDOException $e) {
24+
echo $e::class, ': ', $e->getCode(), PHP_EOL;
25+
}
26+
27+
$db->beginTransaction();
28+
unset($stmt);
29+
30+
$db->exec('SELECT 2');
31+
32+
echo 'Done', PHP_EOL;
33+
34+
?>
35+
--EXPECT--
36+
PDOException: 22P02
37+
Done

‎ext/pdo_pgsql/tests/cursor_scroll_reexecute_after_rollback.phpt‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
PDO PgSQL PDO::CURSOR_SCROLL re-execute after a rollback destroyed the cursor
3+
--EXTENSIONS--
4+
pdo_pgsql
5+
--SKIPIF--
6+
<?php
7+
require __DIR__ . '/config.inc';
8+
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
9+
PDOTest::skip();
10+
?>
11+
--FILE--
12+
<?php
13+
14+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
15+
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
16+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
17+
18+
$db->beginTransaction();
19+
20+
$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
21+
$stmt->execute();
22+
23+
$db->rollBack();
24+
25+
$db->beginTransaction();
26+
27+
$stmt->execute();
28+
echo $stmt->fetchColumn(), PHP_EOL;
29+
30+
echo $db->query('SELECT 2')->fetchColumn(), PHP_EOL;
31+
32+
$db->rollBack();
33+
34+
echo 'Done', PHP_EOL;
35+
36+
?>
37+
--EXPECT--
38+
1
39+
2
40+
Done

‎ext/pdo_pgsql/tests/cursor_scroll_rollback_destroyed.phpt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE for a cursor a rollback already destroyed
3+
--EXTENSIONS--
4+
pdo_pgsql
5+
--SKIPIF--
6+
<?php
7+
require __DIR__ . '/config.inc';
8+
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
9+
PDOTest::skip();
10+
?>
11+
--FILE--
12+
<?php
13+
14+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
15+
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
16+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
17+
18+
$db->beginTransaction();
19+
20+
$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
21+
$stmt->execute();
22+
23+
$db->rollBack();
24+
25+
$db->beginTransaction();
26+
unset($stmt);
27+
28+
$db->exec('SELECT 2');
29+
30+
echo 'Done', PHP_EOL;
31+
32+
?>
33+
--EXPECT--
34+
Done

‎ext/pdo_pgsql/tests/cursor_scroll_without_declare.phpt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
PDO PgSQL PDO::CURSOR_SCROLL sends no CLOSE for a cursor it never declared
3+
--EXTENSIONS--
4+
pdo_pgsql
5+
--SKIPIF--
6+
<?php
7+
require __DIR__ . '/config.inc';
8+
require dirname(__DIR__, 2) . '/pdo/tests/pdo_test.inc';
9+
PDOTest::skip();
10+
?>
11+
--FILE--
12+
<?php
13+
14+
require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
15+
$db = PDOTest::test_factory(__DIR__ . '/common.phpt');
16+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
17+
18+
$db->beginTransaction();
19+
20+
$stmt = $db->prepare('SELECT 1', [PDO::ATTR_CURSOR => PDO::CURSOR_SCROLL]);
21+
unset($stmt);
22+
23+
$db->exec('SELECT 2');
24+
25+
echo 'Done';
26+
27+
?>
28+
--EXPECT--
29+
Done

0 commit comments

Comments
 (0)