Skip to content

Commit a86b3d2

Browse files
KentarouTakedaKentarouTakeda
committed
ext/pdo_pgsql: Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist
1 parent c54c15c commit a86b3d2

10 files changed

Lines changed: 287 additions & 9 deletions

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ PHP NEWS
7676
. Fixed bug GH-23444 (ODBC_ATTR_ASSUME_UTF8 corrupts Unicode data outside
7777
Windows). (Calvin Buckley, Lazizbek Ergashev)
7878

79+
- PDO_PGSQL:
80+
. Fixed PDO::CURSOR_SCROLL statements closing a cursor that does not exist.
81+
(KentarouTakeda)
82+
7983
- PDO Sqlite:
8084
. Fixed bug GH-20214 (PDO::FETCH_DEFAULT unexpected behavior with
8185
PDOStatement::setFetchMode). (SakiTakamachi)

‎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
PHP_CHECK_PDO_INCLUDES
2935

3036
PHP_NEW_EXTENSION([pdo_pgsql],

‎ext/pdo_pgsql/pgsql_statement.c‎

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,43 @@
5656
#define FLOAT8LABEL "float8"
5757
#define FLOAT8OID 701
5858

59+
#ifndef HAVE_PQCLOSEPORTAL
60+
static bool pdo_pgsql_try_cmd(const char *cmd, pdo_pgsql_db_handle *H)
61+
{
62+
bool result = false;
63+
char *q = NULL;
64+
PGresult *res = NULL;
65+
66+
PGTransactionStatusType status = PQtransactionStatus(H->server);
67+
68+
switch (status) {
69+
case PQTRANS_ACTIVE:
70+
case PQTRANS_INERROR:
71+
break;
72+
case PQTRANS_INTRANS: /* failure must not abort the caller's transaction */
73+
/* PQexec does not run the statements following a failed one */
74+
spprintf(&q, 0, "SAVEPOINT pdo_pgsql_savepoint; %s; RELEASE SAVEPOINT pdo_pgsql_savepoint;", cmd);
75+
res = PQexec(H->server, q);
76+
77+
if (PQresultStatus(res) != PGRES_COMMAND_OK) {
78+
PQclear(PQexec(H->server, "ROLLBACK TO SAVEPOINT pdo_pgsql_savepoint; RELEASE SAVEPOINT pdo_pgsql_savepoint"));
79+
}
5980

81+
break;
82+
default:
83+
res = PQexec(H->server, cmd);
84+
}
85+
86+
if (PQresultStatus(res) == PGRES_COMMAND_OK) {
87+
result = true;
88+
}
89+
90+
if (q) efree(q);
91+
if (res) PQclear(res);
92+
93+
return result;
94+
}
95+
#endif
6096

6197
static int pgsql_stmt_dtor(pdo_stmt_t *stmt)
6298
{
@@ -114,15 +150,16 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt)
114150
}
115151

116152
if (S->cursor_name) {
117-
if (server_obj_usable) {
153+
if (S->is_cursor_declared && server_obj_usable) {
118154
pdo_pgsql_db_handle *H = S->H;
119-
char *q = NULL;
120-
PGresult *res;
121-
155+
#ifndef HAVE_PQCLOSEPORTAL
156+
char *q;
122157
spprintf(&q, 0, "CLOSE %s", S->cursor_name);
123-
res = PQexec(H->server, q);
158+
pdo_pgsql_try_cmd(q, H);
124159
efree(q);
125-
if (res) PQclear(res);
160+
#else
161+
PQclear(PQclosePortal(H->server, S->cursor_name));
162+
#endif
126163
}
127164
efree(S->cursor_name);
128165
S->cursor_name = NULL;
@@ -156,10 +193,19 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt)
156193
if (S->cursor_name) {
157194
char *q = NULL;
158195

159-
if (S->is_prepared) {
196+
if (S->is_cursor_declared) {
197+
#ifndef HAVE_PQCLOSEPORTAL
160198
spprintf(&q, 0, "CLOSE %s", S->cursor_name);
161-
PQclear(PQexec(H->server, q));
199+
200+
if (pdo_pgsql_try_cmd(q, H)) {
201+
S->is_cursor_declared = false;
202+
}
203+
162204
efree(q);
205+
#else
206+
PQclear(PQclosePortal(H->server, S->cursor_name));
207+
S->is_cursor_declared = false;
208+
#endif
163209
}
164210

165211
spprintf(&q, 0, "DECLARE %s SCROLL CURSOR WITH HOLD FOR %s", S->cursor_name, ZSTR_VAL(stmt->active_query_string));
@@ -175,7 +221,7 @@ static int pgsql_stmt_execute(pdo_stmt_t *stmt)
175221
PQclear(S->result);
176222

177223
/* the cursor was declared correctly */
178-
S->is_prepared = 1;
224+
S->is_cursor_declared = true;
179225

180226
/* fetch to be able to get the number of tuples later, but don't advance the cursor pointer */
181227
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
@@ -68,6 +68,7 @@ typedef struct {
6868
Oid *param_types;
6969
int current_row;
7070
bool is_prepared;
71+
bool is_cursor_declared;
7172
} pdo_pgsql_stmt;
7273

7374
typedef struct {

‎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)