ext/pdo_pgsql: Fix the connection state left behind by a lazy fetch

Author: KentarouTakeda

NEWS

@@ -41,6 +41,14 @@ PHP                                                                        NEWS
   . Fixed bug GH-23016 (NULL values in long columns come back as garbage
     binary strings). (Calvin Buckley, iliaal)
 
+- PDO_PGSQL:
+  . Fixed a lazy fetch (PDO::ATTR_PREFETCH => 0) leaving the connection busy
+    for the next one when no prepared statement is used. (KentarouTakeda)
+  . Fixed a use-after-free after destroying a statement when no prepared
+    statement is used. (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

@@ -71,7 +71,6 @@ static void pgsql_stmt_finish(pdo_pgsql_stmt *S, int fin_mode)
 		char errbuf[256];
 		PQcancel(cancel, errbuf, 256);
 		PQfreeCancel(cancel);
-		S->is_running_unbuffered = false;
 	}
 
 	if (S->result) {
@@ -113,9 +112,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 +122,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 +590,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,37 @@
+--TEST--
+PDO PgSQL an abandoned lazy fetch frees the connection without a prepared statement
+--EXTENSIONS--
+pdo
+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_drain.phpt

@@ -0,0 +1,37 @@
+--TEST--
+PDO PgSQL a drained lazy fetch frees the connection without a prepared statement
+--EXTENSIONS--
+pdo
+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,29 @@
+--TEST--
+PDO PgSQL a lazy fetch whose stream was taken over reports no leftover rows
+--EXTENSIONS--
+pdo
+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) {
+}