[php-src] master: ext/pdo: Clear bindings when execute() fails to bind its array (#23970)

Author: Ilia Alshanetsky (iliaal) Committer: GitHub (web-flow) Pusher: iliaal Date: 2026-10-24T19:25:22-04:00 Commit: https://github.com/php/php-src/commit/17c806d8b341ca3c67fd56a3a9594c28a4cd5b51 Raw diff: https://github.com/php/php-src/commit/17c806d8b341ca3c67fd56a3a9594c28a4cd5b51.diff ext/pdo: Clear bindings when execute() fails to bind its array (#23970) PDOStatement::execute() destroyed bound_params before registering the entries of its array argument, so a failure part-way through (a value whose string conversion throws) left the entries registered so far in place, and a following execute() without arguments ran with them. Drop the partial table on failure instead. really_register_bound_param() now looks the table up only after converting the value, and raises the PDO_PARAM_EVT_ALLOC error after undoing its insert, so a __toString() or error handler that re-executes the statement no longer frees the table in use. Changed paths: A ext/pdo/tests/pdo_bind_reentrant_execute.phpt A ext/pdo/tests/pdo_execute_array_binding_failure.phpt M NEWS M ext/pdo/pdo_stmt.c Diff: diff --git a/NEWS b/NEWS index 6bb8496271f8..efec2b84e882 100644 --- a/NEWS +++ b/NEWS @@ -87,6 +87,8 @@ PHP NEWS that is not in the result set. (Ilia Alshanetsky) . Fixed bug GH-23962 (Destroying a persistent PDO instance rolls back a transaction still in use by another instance). (Lazizbek Ergashev) + . Fixed PDOStatement::execute() leaving a partial set of bindings in place + when binding its array argument fails. (Ilia Alshanetsky, Kamil Tekiela) - PDO_PGSQL: . Improved performance of the SQL parser that handles placeholders. diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 2a84d7ae2e98..7f39a2d57029 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -260,19 +260,6 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_ zval *parameter; struct pdo_bound_param_data *pparam = NULL; - hash = is_param ? stmt->bound_params : stmt->bound_columns; - - if (!hash) { - ALLOC_HASHTABLE(hash); - zend_hash_init(hash, 13, NULL, param_dtor, 0); - - if (is_param) { - stmt->bound_params = hash; - } else { - stmt->bound_columns = hash; - } - } - if (!Z_ISREF(param->parameter)) { parameter = &param->parameter; } else { @@ -348,6 +335,17 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_ /* delete any other parameter registered with this number. * If the parameter is named, it will be removed and correctly * disposed of by the hash_update call that follows */ + hash = is_param ? stmt->bound_params : stmt->bound_columns; + if (!hash) { + ALLOC_HASHTABLE(hash); + zend_hash_init(hash, 13, NULL, param_dtor, 0); + if (is_param) { + stmt->bound_params = hash; + } else { + stmt->bound_columns = hash; + } + } + if (param->paramno >= 0) { zend_hash_index_del(hash, param->paramno); } @@ -362,7 +360,6 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_ /* tell the driver we just created a parameter */ if (stmt->methods->param_hook) { if (!stmt->methods->param_hook(stmt, pparam, PDO_PARAM_EVT_ALLOC)) { - PDO_HANDLE_STMT_ERR(); /* undo storage allocation; the hash will free the parameter * name if required */ if (pparam->name) { @@ -373,6 +370,7 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_ ZVAL_UNDEF(&param->driver_params); /* param->parameter is freed by hash dtor */ ZVAL_UNDEF(&param->parameter); + PDO_HANDLE_STMT_ERR(); return false; } } @@ -434,6 +432,11 @@ PHP_METHOD(PDOStatement, execute) if (!Z_ISUNDEF(param.parameter)) { zval_ptr_dtor(&param.parameter); } + if (stmt->bound_params) { + zend_hash_destroy(stmt->bound_params); + FREE_HASHTABLE(stmt->bound_params); + stmt->bound_params = NULL; + } RETURN_FALSE; } } ZEND_HASH_FOREACH_END(); diff --git a/ext/pdo/tests/pdo_bind_reentrant_execute.phpt b/ext/pdo/tests/pdo_bind_reentrant_execute.phpt new file mode 100644 index 000000000000..dfc9fdab8314 --- /dev/null +++ b/ext/pdo/tests/pdo_bind_reentrant_execute.phpt @@ -0,0 +1,42 @@ +--TEST-- +PDO: re-executing the statement while a bound value is converted to string +--EXTENSIONS-- +pdo +--SKIPIF-- +<?php +$dir = getenv('REDIR_TEST_DIR'); +if (false == $dir) die('skip no driver'); +require_once $dir . 'pdo_test.inc'; +PDOTest::skip(); +?> +--FILE-- +<?php +if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/'); +require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; + +class ReExecute +{ + public function __toString(): string + { + global $stmt; + $stmt->execute(['x', 'y']); + return 'r'; + } +} + +$db = PDOTest::factory(); +$db->exec('CREATE TABLE test_bind_reentrant (name varchar(10))'); +$stmt = $db->prepare('SELECT name FROM test_bind_reentrant WHERE name = ? OR name = ?'); + +$stmt->execute(['a', new ReExecute()]); +$stmt->bindValue(2, new ReExecute()); +echo "Done\n"; +?> +--CLEAN-- +<?php +require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; +$db = PDOTest::factory(); +PDOTest::dropTableIfExists($db, 'test_bind_reentrant'); +?> +--EXPECT-- +Done diff --git a/ext/pdo/tests/pdo_execute_array_binding_failure.phpt b/ext/pdo/tests/pdo_execute_array_binding_failure.phpt new file mode 100644 index 000000000000..f247973874b8 --- /dev/null +++ b/ext/pdo/tests/pdo_execute_array_binding_failure.phpt @@ -0,0 +1,67 @@ +--TEST-- +PDO: execute() leaves no bindings when binding its array argument fails +--EXTENSIONS-- +pdo +--SKIPIF-- +<?php +$dir = getenv('REDIR_TEST_DIR'); +if (false == $dir) die('skip no driver'); +require_once $dir . 'pdo_test.inc'; +PDOTest::skip(); +?> +--FILE-- +<?php +if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/'); +require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; + +class ThrowingString +{ + public function __toString(): string + { + throw new RuntimeException('conversion failed'); + } +} + +function bound_params_count(PDOStatement $stmt): string +{ + ob_start(); + $stmt->debugDumpParams(); + preg_match('/^Params:\s+(\d+)$/m', ob_get_clean(), $m); + return $m[1]; +} + +$db = PDOTest::factory(); +$db->exec('CREATE TABLE test_execute_bind_fail (id int, name varchar(10))'); +$db->exec("INSERT INTO test_execute_bind_fail (id, name) VALUES (1, 'a')"); +$db->exec("INSERT INTO test_execute_bind_fail (id, name) VALUES (2, 'b')"); + +$stmt = $db->prepare('SELECT name FROM test_execute_bind_fail WHERE id = :id AND name = :name'); +$id = 1; +$name = 'a'; +$stmt->bindParam(':id', $id); +$stmt->bindParam(':name', $name); + +try { + $stmt->execute([':id' => 2, ':name' => new ThrowingString()]); +} catch (RuntimeException $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} +echo "bound params after failure: ", bound_params_count($stmt), PHP_EOL; + +var_dump($stmt->execute([':id' => 2, ':name' => 'b'])); +var_dump($stmt->fetchAll(PDO::FETCH_COLUMN)); +?> +--CLEAN-- +<?php +require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; +$db = PDOTest::factory(); +PDOTest::dropTableIfExists($db, 'test_execute_bind_fail'); +?> +--EXPECT-- +RuntimeException: conversion failed +bound params after failure: 0 +bool(true) +array(1) { + [0]=> + string(1) "b" +}