diff --git a/NEWS b/NEWS index 01849a378fff..247c3aa7e6ea 100644 --- a/NEWS +++ b/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) diff --git a/ext/pdo_pgsql/pgsql_statement.c b/ext/pdo_pgsql/pgsql_statement.c index 89f713ffcbff..3fb3e9444830 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; @@ -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 000000000000..7968c2653206 --- /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-- + +--FILE-- +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 000000000000..91321e2bcde2 --- /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-- + +--FILE-- +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 000000000000..a650628d3cce --- /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-- + +--FILE-- +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 000000000000..eace678310de --- /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-- + +--FILE-- +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) { +}