Skip to content

Commit 3847ee1

Browse files
iliaaliliaal
committed
ext/pdo: Keep prior bindings when execute() fails to bind its array
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, a name the driver rejects) left the entries registered so far in place, and a following execute() without arguments ran with them. Build the new table separately and install it only once every entry is registered. This also stops a value's __toString() that re-executes the statement from freeing the table being filled.
1 parent ed60b9e commit 3847ee1

3 files changed

Lines changed: 91 additions & 20 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ PHP NEWS
8484
column index. (Ilia Alshanetsky)
8585
. Fixed PDOStatement::bindColumn() registering a binding for a column name
8686
that is not in the result set. (Ilia Alshanetsky)
87+
. Fixed PDOStatement::execute() leaving a partial set of bindings in place
88+
when binding its array argument fails. (Ilia Alshanetsky)
8789

8890
- Readline:
8991
. Fixed a heap over-read in the interactive shell prompt when cli.prompt is

‎ext/pdo/pdo_stmt.c‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -249,22 +249,22 @@ static void param_dtor(zval *el) /* {{{ */
249249
}
250250
/* }}} */
251251

252-
static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_stmt_t *stmt, bool is_param) /* {{{ */
252+
static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_stmt_t *stmt, bool is_param, HashTable *hash)
253253
{
254-
HashTable *hash;
255254
zval *parameter;
256255
struct pdo_bound_param_data *pparam = NULL;
257256

258-
hash = is_param ? stmt->bound_params : stmt->bound_columns;
259-
260257
if (!hash) {
261-
ALLOC_HASHTABLE(hash);
262-
zend_hash_init(hash, 13, NULL, param_dtor, 0);
258+
hash = is_param ? stmt->bound_params : stmt->bound_columns;
259+
if (!hash) {
260+
ALLOC_HASHTABLE(hash);
261+
zend_hash_init(hash, 13, NULL, param_dtor, 0);
263262

264-
if (is_param) {
265-
stmt->bound_params = hash;
266-
} else {
267-
stmt->bound_columns = hash;
263+
if (is_param) {
264+
stmt->bound_params = hash;
265+
} else {
266+
stmt->bound_columns = hash;
267+
}
268268
}
269269
}
270270

@@ -381,7 +381,6 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_
381381
}
382382
return 1;
383383
}
384-
/* }}} */
385384

386385
/* {{{ Execute a prepared statement, optionally binding parameters */
387386
PHP_METHOD(PDOStatement, execute)
@@ -402,13 +401,10 @@ PHP_METHOD(PDOStatement, execute)
402401
zval *tmp;
403402
zend_string *key = NULL;
404403
zend_ulong num_index;
404+
HashTable *bound_params = NULL;
405405

406-
if (stmt->bound_params) {
407-
zend_hash_destroy(stmt->bound_params);
408-
FREE_HASHTABLE(stmt->bound_params);
409-
stmt->bound_params = NULL;
410-
}
411-
406+
ALLOC_HASHTABLE(bound_params);
407+
zend_hash_init(bound_params, 13, NULL, param_dtor, 0);
412408
ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(input_params), num_index, key, tmp) {
413409
memset(&param, 0, sizeof(param));
414410

@@ -425,13 +421,21 @@ PHP_METHOD(PDOStatement, execute)
425421
param.param_type = PDO_PARAM_STR;
426422
ZVAL_COPY(&param.parameter, tmp);
427423

428-
if (!really_register_bound_param(&param, stmt, 1)) {
424+
if (!really_register_bound_param(&param, stmt, 1, bound_params)) {
429425
if (!Z_ISUNDEF(param.parameter)) {
430426
zval_ptr_dtor(&param.parameter);
431427
}
428+
zend_hash_destroy(bound_params);
429+
FREE_HASHTABLE(bound_params);
432430
RETURN_FALSE;
433431
}
434432
} ZEND_HASH_FOREACH_END();
433+
434+
if (stmt->bound_params) {
435+
zend_hash_destroy(stmt->bound_params);
436+
FREE_HASHTABLE(stmt->bound_params);
437+
}
438+
stmt->bound_params = bound_params;
435439
}
436440

437441
if (PDO_PLACEHOLDER_NONE == stmt->supports_placeholders) {
@@ -1458,7 +1462,7 @@ static void register_bound_param(INTERNAL_FUNCTION_PARAMETERS, int is_param) /*
14581462
}
14591463

14601464
ZVAL_COPY(&param.parameter, parameter);
1461-
if (!really_register_bound_param(&param, stmt, is_param)) {
1465+
if (!really_register_bound_param(&param, stmt, is_param, NULL)) {
14621466
if (!Z_ISUNDEF(param.parameter)) {
14631467
zval_ptr_dtor(&(param.parameter));
14641468
}
@@ -1502,7 +1506,7 @@ PHP_METHOD(PDOStatement, bindValue)
15021506
}
15031507

15041508
ZVAL_COPY(&param.parameter, parameter);
1505-
if (!really_register_bound_param(&param, stmt, TRUE)) {
1509+
if (!really_register_bound_param(&param, stmt, TRUE, NULL)) {
15061510
if (!Z_ISUNDEF(param.parameter)) {
15071511
zval_ptr_dtor(&(param.parameter));
15081512
ZVAL_UNDEF(&param.parameter);

‎ext/pdo/tests/pdo_execute_array_binding_failure.phpt‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
--TEST--
2+
PDO: execute() keeps the existing bindings when binding its array argument fails
3+
--EXTENSIONS--
4+
pdo
5+
--SKIPIF--
6+
<?php
7+
$dir = getenv('REDIR_TEST_DIR');
8+
if (false == $dir) die('skip no driver');
9+
require_once $dir . 'pdo_test.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.__DIR__ . '/../../pdo/tests/');
15+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
16+
17+
class ThrowingString
18+
{
19+
public function __toString(): string
20+
{
21+
throw new RuntimeException('conversion failed');
22+
}
23+
}
24+
25+
$db = PDOTest::factory();
26+
$db->exec('CREATE TABLE test_execute_bind_fail (id int, name varchar(10))');
27+
$db->exec("INSERT INTO test_execute_bind_fail (id, name) VALUES (1, 'a')");
28+
$db->exec("INSERT INTO test_execute_bind_fail (id, name) VALUES (2, 'b')");
29+
30+
$stmt = $db->prepare('SELECT name FROM test_execute_bind_fail WHERE id = :id AND name = :name');
31+
$id = 1;
32+
$name = 'a';
33+
$stmt->bindParam(':id', $id);
34+
$stmt->bindParam(':name', $name);
35+
36+
try {
37+
$stmt->execute([':id' => 2, ':name' => new ThrowingString()]);
38+
} catch (RuntimeException $e) {
39+
echo $e::class, ": ", $e->getMessage(), PHP_EOL;
40+
}
41+
42+
var_dump($stmt->execute());
43+
var_dump($stmt->fetchAll(PDO::FETCH_COLUMN));
44+
45+
var_dump($stmt->execute([':id' => 2, ':name' => 'b']));
46+
var_dump($stmt->fetchAll(PDO::FETCH_COLUMN));
47+
?>
48+
--CLEAN--
49+
<?php
50+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
51+
$db = PDOTest::factory();
52+
PDOTest::dropTableIfExists($db, 'test_execute_bind_fail');
53+
?>
54+
--EXPECT--
55+
RuntimeException: conversion failed
56+
bool(true)
57+
array(1) {
58+
[0]=>
59+
string(1) "a"
60+
}
61+
bool(true)
62+
array(1) {
63+
[0]=>
64+
string(1) "b"
65+
}

0 commit comments

Comments
 (0)