Commit 0b91828b01e for php.net
commit 0b91828b01e61eef064df7ba4238a469029f39ed
Author: 武田 憲太郎 <takeda@youmind.jp>
Date: Tue Aug 11 19:44:17 2026 +0100
ext/pdo_pgsql: Fix several lazy fetch defects
With PDO::ATTR_PREFETCH => 0 a statement streams its result set, and the
cleanup reads the rest of it by calling PQgetResult() until it returns
NULL. That never happens while the connection is copying: PQgetResult()
hands out a fresh COPY result every time. A COPY run through a lazy fetch
has therefore spun at 100% CPU since 8.5.0, as soon as another lazy fetch
takes the connection over. The copy has to be ended first: a copy in with
PQputCopyEnd(), a copy out by draining PQgetCopyData().
The drain was skipped as well, because is_running_unbuffered was cleared
first, both in the cleanup's own abort path and in pgsql_stmt_fetch()
before it calls the cleanup. With PDO::ATTR_EMULATE_PREPARES or
Pdo\Pgsql::ATTR_DISABLE_PREPARES the connection then stayed busy and the
next lazy fetch failed with "another command is already in progress".
The connection's pointer to the statement streaming on it was only cleared
while closing a server-side prepared statement, which those two modes do
not create, so destroying one left the pointer dangling for the next lazy
fetch to read. And a statement whose stream was taken over kept its row
counters after its result had been freed, so fetch() returned a row of
NULLs rather than false.
Close GH-23065
diff --git a/NEWS b/NEWS
index ffc76ae4a64..ac441555e78 100644
--- a/NEWS
+++ b/NEWS
@@ -55,6 +55,13 @@ PHP NEWS
. Fixed bug GH-23016 (NULL values in long columns come back as garbage
binary strings). (Calvin Buckley, iliaal)
+- PDO_PGSQL:
+ . Fixed several lazy fetch (PDO::ATTR_PREFETCH => 0) defects: an infinite
+ loop when cleaning up a fetch left in a COPY, a use-after-free when a
+ statement with emulated or disabled prepares is destroyed, a connection
+ left busy for the next fetch, and rows delivered from a result another
+ statement took over. (KentarouTakeda)
+
- Reflection:
. Fixed bug GH-22905 (Reflection exception messages truncate on null bytes).
(DanielEScherzer)
diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c
index 89f713ffcbf..be3f31f62a3 100644
--- a/ext/pdo_pgsql/pgsql_statement.c
+++ b/ext/pdo_pgsql/pgsql_statement.c
@@ -66,12 +66,12 @@ static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
{
pdo_pgsql_db_handle *H = S->H;
- if (S->is_running_unbuffered && S->result && (fin_mode & FIN_ABORT)) {
+ /* a buffered query may have already drained this statement's stream */
+ if (S->is_running_unbuffered && H->running_stmt == S && S->result && (fin_mode & FIN_ABORT)) {
PGcancel *cancel = PQgetCancel(H->server);
char errbuf[256];
PQcancel(cancel, errbuf, 256);
PQfreeCancel(cancel);
- S->is_running_unbuffered = false;
}
if (S->result) {
@@ -80,7 +80,7 @@ static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
S->result = NULL;
}
- if (S->is_running_unbuffered) {
+ if (S->is_running_unbuffered && H->running_stmt == S) {
/* https://postgresql.org/docs/current/libpq-async.html:
* "PQsendQuery cannot be called again until PQgetResult has returned NULL"
* And as all single-row functions are connection-wise instead of statement-wise,
@@ -90,8 +90,35 @@ static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
// instead of discarding results we could store them to their statement
// so that their fetch() will get them (albeit not in lazy mode anymore).
while ((S->result = PQgetResult(H->server))) {
+ ExecStatusType status = PQresultStatus(S->result);
+
PQclear(S->result);
S->result = NULL;
+
+ /* PQgetResult() keeps handing out the same result while the
+ * connection is copying: only these calls can end it */
+ if (status == PGRES_COPY_IN || status == PGRES_COPY_BOTH) {
+ /* fail a copy in, so that abandoning a statement cannot
+ * commit it; a replication stream only accepts a clean end */
+ const char *error = status == PGRES_COPY_IN
+ ? "COPY terminated by PDO"
+ : NULL;
+
+ if (PQputCopyEnd(H->server, error) < 0) {
+ break;
+ }
+ }
+ if (status == PGRES_COPY_OUT || status == PGRES_COPY_BOTH) {
+ char *buf;
+ int nbytes;
+
+ while ((nbytes = PQgetCopyData(H->server, &buf, 0)) > 0) {
+ PQfreemem(buf);
+ }
+ if (nbytes < -1) {
+ break;
+ }
+ }
}
S->is_running_unbuffered = false;
}
@@ -113,9 +140,6 @@ static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
}
S->is_prepared = false;
- if (H->running_stmt == S) {
- H->running_stmt = NULL;
- }
}
}
@@ -126,6 +150,10 @@ static int pgsql_stmt_dtor(pdo_stmt_t *stmt)
pgsql_stmt_finish(S, FIN_DISCARD|(server_obj_usable ? FIN_CLOSE|FIN_ABORT : 0));
+ if (server_obj_usable && S->H->running_stmt == S) {
+ S->H->running_stmt = NULL;
+ }
+
if (S->stmt_name) {
efree(S->stmt_name);
S->stmt_name = NULL;
@@ -561,7 +589,7 @@ static int pgsql_stmt_fetch(pdo_stmt_t *stmt,
return 0;
}
} else {
- if (S->is_running_unbuffered && S->current_row >= stmt->row_count) {
+ if (S->is_running_unbuffered && S->H->running_stmt == S && S->current_row >= stmt->row_count) {
ExecStatusType status;
/* @todo in unbuffered mode, PQ allows multiple queries to be passed:
@@ -590,12 +618,12 @@ static int pgsql_stmt_fetch(pdo_stmt_t *stmt,
S->current_row = 0;
if (!stmt->row_count) {
- S->is_running_unbuffered = false;
/* libpq requires looping until getResult returns null */
pgsql_stmt_finish(S, 0);
}
}
- if (S->current_row < stmt->row_count) {
+ /* another statement may have taken over and freed the result */
+ if (S->result && S->current_row < stmt->row_count) {
S->current_row++;
return 1;
} else {
diff --git a/ext/pdo_pgsql/tests/lazy_fetch_cancel.phpt b/ext/pdo_pgsql/tests/lazy_fetch_cancel.phpt
new file mode 100644
index 00000000000..7968c265320
--- /dev/null
+++ b/ext/pdo_pgsql/tests/lazy_fetch_cancel.phpt
@@ -0,0 +1,36 @@
+--TEST--
+PDO PgSQL an abandoned lazy fetch frees the connection without a prepared statement
+--EXTENSIONS--
+pdo_pgsql
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+$pdo = PDOTest::test_factory(__DIR__ . '/common.phpt');
+$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+
+foreach ([
+ 'PDO::ATTR_EMULATE_PREPARES' => [PDO::ATTR_EMULATE_PREPARES => true],
+ 'Pdo\Pgsql::ATTR_DISABLE_PREPARES' => [Pdo\Pgsql::ATTR_DISABLE_PREPARES => true],
+] as $label => $options) {
+ $options[PDO::ATTR_PREFETCH] = 0;
+
+ $stmt = $pdo->prepare("VALUES (1), (2)", $options);
+ $stmt->execute();
+ $stmt = null;
+
+ $stmt = $pdo->prepare("VALUES (1), (2)", $options);
+ $stmt->execute();
+ echo "$label: ";
+ var_dump((bool) $stmt->fetchAll());
+}
+?>
+--EXPECT--
+PDO::ATTR_EMULATE_PREPARES: bool(true)
+Pdo\Pgsql::ATTR_DISABLE_PREPARES: bool(true)
diff --git a/ext/pdo_pgsql/tests/lazy_fetch_copy.phpt b/ext/pdo_pgsql/tests/lazy_fetch_copy.phpt
new file mode 100644
index 00000000000..91321e2bcde
--- /dev/null
+++ b/ext/pdo_pgsql/tests/lazy_fetch_copy.phpt
@@ -0,0 +1,35 @@
+--TEST--
+PDO PgSQL a lazy fetch left in a COPY does not hang the connection cleanup
+--EXTENSIONS--
+pdo_pgsql
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+$pdo = PDOTest::test_factory(__DIR__ . '/common.phpt');
+$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT);
+$pdo->setAttribute(PDO::ATTR_PREFETCH, 0);
+$pdo->exec("CREATE TEMPORARY TABLE lazy_fetch_copy (i int)");
+
+foreach ([
+ 'COPY OUT' => "COPY (SELECT 1) TO STDOUT",
+ 'COPY IN' => "COPY lazy_fetch_copy FROM STDIN",
+] as $label => $sql) {
+ $copy = $pdo->prepare($sql);
+ $copy->execute();
+
+ $stmt = $pdo->prepare("VALUES (1), (2)");
+ $stmt->execute();
+ echo "$label: ";
+ var_dump((bool) $stmt->fetchAll());
+}
+?>
+--EXPECT--
+COPY OUT: bool(true)
+COPY IN: bool(true)
diff --git a/ext/pdo_pgsql/tests/lazy_fetch_drain.phpt b/ext/pdo_pgsql/tests/lazy_fetch_drain.phpt
new file mode 100644
index 00000000000..a650628d3cc
--- /dev/null
+++ b/ext/pdo_pgsql/tests/lazy_fetch_drain.phpt
@@ -0,0 +1,36 @@
+--TEST--
+PDO PgSQL a drained lazy fetch frees the connection without a prepared statement
+--EXTENSIONS--
+pdo_pgsql
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+$pdo = PDOTest::test_factory(__DIR__ . '/common.phpt');
+$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+
+foreach ([
+ 'PDO::ATTR_EMULATE_PREPARES' => [PDO::ATTR_EMULATE_PREPARES => true],
+ 'Pdo\Pgsql::ATTR_DISABLE_PREPARES' => [Pdo\Pgsql::ATTR_DISABLE_PREPARES => true],
+] as $label => $options) {
+ $options[PDO::ATTR_PREFETCH] = 0;
+
+ $stmt = $pdo->prepare("VALUES (1), (2)", $options);
+ $stmt->execute();
+ $stmt->fetchAll();
+
+ $stmt = $pdo->prepare("VALUES (1), (2)", $options);
+ $stmt->execute();
+ echo "$label: ";
+ var_dump((bool) $stmt->fetchAll());
+}
+?>
+--EXPECT--
+PDO::ATTR_EMULATE_PREPARES: bool(true)
+Pdo\Pgsql::ATTR_DISABLE_PREPARES: bool(true)
diff --git a/ext/pdo_pgsql/tests/lazy_fetch_takeover.phpt b/ext/pdo_pgsql/tests/lazy_fetch_takeover.phpt
new file mode 100644
index 00000000000..eace678310d
--- /dev/null
+++ b/ext/pdo_pgsql/tests/lazy_fetch_takeover.phpt
@@ -0,0 +1,28 @@
+--TEST--
+PDO PgSQL a lazy fetch whose stream was taken over reports no leftover rows
+--EXTENSIONS--
+pdo_pgsql
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+$pdo = PDOTest::test_factory(__DIR__ . '/common.phpt');
+$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+$pdo->setAttribute(PDO::ATTR_PREFETCH, 0);
+
+$first = $pdo->prepare("VALUES (1), (2)");
+$first->execute();
+
+$pdo->prepare("VALUES (1), (2)")->execute();
+
+var_dump($first->fetchAll(PDO::FETCH_NUM));
+?>
+--EXPECT--
+array(0) {
+}
diff --git a/ext/pdo_pgsql/tests/lazy_fetch_takeover_buffered.phpt b/ext/pdo_pgsql/tests/lazy_fetch_takeover_buffered.phpt
new file mode 100644
index 00000000000..aa8c2aa86c4
--- /dev/null
+++ b/ext/pdo_pgsql/tests/lazy_fetch_takeover_buffered.phpt
@@ -0,0 +1,38 @@
+--TEST--
+PDO PgSQL a lazy fetch stale after a buffered query does not read the next statement's rows
+--EXTENSIONS--
+pdo_pgsql
+--SKIPIF--
+<?php
+require __DIR__ . '/config.inc';
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+
+require __DIR__ . '/../../../ext/pdo/tests/pdo_test.inc';
+$pdo = PDOTest::test_factory(__DIR__ . '/common.phpt');
+$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
+
+$first = $pdo->prepare("VALUES (1), (2), (3)", [PDO::ATTR_PREFETCH => 0]);
+$first->execute();
+$first->fetch();
+
+// a buffered query drains the stream but does not end $first's lazy fetch
+$pdo->prepare("VALUES (99)")->execute();
+
+$third = $pdo->prepare("VALUES (777), (888)", [PDO::ATTR_PREFETCH => 0]);
+$third->execute();
+
+var_dump($first->fetch(PDO::FETCH_NUM));
+var_dump($third->fetchAll(PDO::FETCH_COLUMN));
+?>
+--EXPECT--
+bool(false)
+array(2) {
+ [0]=>
+ string(3) "777"
+ [1]=>
+ string(3) "888"
+}