ext/pdo_pgsql: Fix several lazy fetch defects

NEWS

@@ -43,6 +43,16 @@ PHP                                                                        NEWS
   . Fixed bug GH-23016 (NULL values in long columns come back as garbage
     binary strings). (Calvin Buckley, iliaal)
 
+- PDO_PGSQL:
+  . Fixed an infinite loop when cleaning up a lazy fetch
+    (PDO::ATTR_PREFETCH => 0) left in a COPY. (KentarouTakeda)
+  . Fixed a use-after-free when a lazy statement with emulated or disabled
+    prepares is destroyed. (KentarouTakeda)
+  . Fixed a lazy fetch with emulated or disabled prepares leaving the
+    connection busy for the next one. (KentarouTakeda)
+  . Fixed a lazy fetch returning a row of NULLs after another statement
+    took over the connection. (KentarouTakeda)
+
 - Reflection:
   . Fixed bug GH-22905 (Reflection exception messages truncate on null bytes).
     (DanielEScherzer)

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;
@@ -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 {

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)

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)

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)

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) {
+}