diff --git a/NEWS b/NEWS index b38484a71ac1..f16ddfab549c 100644 --- a/NEWS +++ b/NEWS @@ -43,6 +43,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) - PGSQL: . Fixed pg_lo_write() rejecting data containing null bytes. (Ilia Alshanetsky) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index c7cf92cfdfa8..1126b67eff09 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -249,19 +249,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 = ¶m->parameter; } else { @@ -346,6 +333,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); } @@ -360,7 +358,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) { @@ -370,6 +367,7 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_ } /* param->parameter is freed by hash dtor */ ZVAL_UNDEF(¶m->parameter); + PDO_HANDLE_STMT_ERR(); return false; } } @@ -423,6 +421,11 @@ PHP_METHOD(PDOStatement, execute) if (!Z_ISUNDEF(param.parameter)) { zval_ptr_dtor(¶m.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-- + +--FILE-- +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-- + +--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-- + +--FILE-- +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-- + +--EXPECT-- +RuntimeException: conversion failed +bound params after failure: 0 +bool(true) +array(1) { + [0]=> + string(1) "b" +}