[php-src] PHP-8.5: ext/pdo: Make PDOStatement::setFetchMode() updates atomic

Author: Ilia Alshanetsky (iliaal) Date: 2026-09-30T13:53:34-04:00 Commit: https://github.com/php/php-src/commit/357fcf12f4e9a610745d1922c93ab795909e90a2 Raw diff: https://github.com/php/php-src/commit/357fcf12f4e9a610745d1922c93ab795909e90a2.diff ext/pdo: Make PDOStatement::setFetchMode() updates atomic pdo_stmt_setup_fetch_mode() cleared the active fetch state before validating the replacement, so a failed call lost FETCH_INTO objects and FETCH_CLASS constructor arguments, and autoload re-entry could observe a mismatched tagged union. Validate the candidate in local storage and replace the active payload only after validation succeeds. Closes GH-23188 Changed paths: A ext/pdo/tests/pdo_setfetchmode_atomic.phpt A ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt A ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt M NEWS M ext/pdo/pdo_stmt.c Diff: diff --git a/NEWS b/NEWS index 1e1f3f6c8fed..9aa7771c056f 100644 --- a/NEWS +++ b/NEWS @@ -137,6 +137,8 @@ PHP NEWS options value. (Ilia Alshanetsky) . Fixed PDOStatement not reporting its bound parameters and columns to the cycle collector. (Ilia Alshanetsky) + . Fixed PDOStatement::setFetchMode() not applying changes atomically. + (Ilia Alshanetsky) - PDO_DBLIB: . Fixed bug GH-23741 (segfault after a failed query inside a PDO diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 9afdc52a2290..b5b743514191 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1772,19 +1772,16 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a uint32_t arg1_arg_num = mode_arg_num + 1; uint32_t constructor_arg_num = mode_arg_num + 2; uint32_t total_num_args = mode_arg_num + variadic_num_args; + zend_long fetch_type = mode & ~PDO_FETCH_FLAGS; + zend_long fetch_column = 0; + zend_class_entry *fetch_class = NULL; + zend_array *fetch_ctor_args = NULL; + zend_object *fetch_into = NULL; + zval old_ctor_args; + zval old_into; - switch (stmt->default_fetch_type) { - case PDO_FETCH_INTO: - if (!Z_ISUNDEF(stmt->fetch.into)) { - zval_ptr_dtor(&stmt->fetch.into); - ZVAL_UNDEF(&stmt->fetch.into); - } - break; - default: - ; - } - - stmt->default_fetch_type = stmt->dbh->default_fetch_type; + ZVAL_UNDEF(&old_ctor_args); + ZVAL_UNDEF(&old_into); flags = mode & PDO_FETCH_FLAGS; @@ -1792,9 +1789,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a return false; } - bool use_default = (mode & ~PDO_FETCH_FLAGS) == PDO_FETCH_USE_DEFAULT; - - switch (mode & ~PDO_FETCH_FLAGS) { + switch (fetch_type) { case PDO_FETCH_USE_DEFAULT: case PDO_FETCH_LAZY: case PDO_FETCH_ASSOC: @@ -1829,13 +1824,11 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a zend_argument_value_error(arg1_arg_num, "must be greater than or equal to 0"); return false; } - stmt->fetch.column = Z_LVAL(args[0]); + fetch_column = Z_LVAL(args[0]); break; case PDO_FETCH_CLASS: { HashTable *constructor_args = NULL; - /* Undef constructor arguments */ - ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); /* Gets its class name from 1st column */ if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) { if (variadic_num_args != 0) { @@ -1845,7 +1838,6 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a zend_string_release(func); return false; } - stmt->fetch.cls.ce = NULL; } else { zend_class_entry *cep; if (variadic_num_args == 0) { @@ -1884,15 +1876,17 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a constructor_args = Z_ARRVAL(args[1]); } } - stmt->fetch.cls.ce = cep; + fetch_class = cep; /* If constructor arguments are present and not empty */ if (constructor_args) { - ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(constructor_args)); + if (!cep->constructor) { + zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments"); + return false; + } + fetch_ctor_args = zend_array_dup(constructor_args); } } - - do_fetch_class_prepare(stmt); break; } case PDO_FETCH_INTO: @@ -1908,17 +1902,57 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a return false; } - ZVAL_COPY(&stmt->fetch.into, &args[0]); + fetch_into = Z_OBJ(args[0]); break; default: zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants"); return false; } - if (!use_default) { + if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_INTO) { + ZVAL_COPY_VALUE(&old_into, &stmt->fetch.into); + ZVAL_UNDEF(&stmt->fetch.into); + } else if ((stmt->default_fetch_type & ~PDO_FETCH_FLAGS) == PDO_FETCH_CLASS) { + do_fetch_opt_finish(stmt, 0); + ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args); + ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); + } else { + do_fetch_opt_finish(stmt, 1); + } + + switch (fetch_type) { + case PDO_FETCH_COLUMN: + stmt->fetch.column = fetch_column; + break; + case PDO_FETCH_CLASS: + stmt->fetch.cls.ce = fetch_class; + if (fetch_ctor_args) { + ZVAL_ARR(&stmt->fetch.cls.ctor_args, fetch_ctor_args); + } else { + ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); + } + do_fetch_class_prepare(stmt); + break; + case PDO_FETCH_INTO: + ZVAL_OBJ_COPY(&stmt->fetch.into, fetch_into); + break; + default: + break; + } + + if (fetch_type == PDO_FETCH_USE_DEFAULT) { + stmt->default_fetch_type = stmt->dbh->default_fetch_type; + } else { stmt->default_fetch_type = mode; } + if (!Z_ISUNDEF(old_into)) { + zval_ptr_dtor(&old_into); + } + if (!Z_ISUNDEF(old_ctor_args)) { + zval_ptr_dtor(&old_ctor_args); + } + return true; } @@ -1934,8 +1968,6 @@ PHP_METHOD(PDOStatement, setFetchMode) PHP_STMT_GET_OBJ; - do_fetch_opt_finish(stmt, 1); - if (!pdo_stmt_setup_fetch_mode(stmt, fetch_mode, 1, args, num_args)) { RETURN_THROWS(); } diff --git a/ext/pdo/tests/pdo_setfetchmode_atomic.phpt b/ext/pdo/tests/pdo_setfetchmode_atomic.phpt new file mode 100644 index 000000000000..8751c27673a5 --- /dev/null +++ b/ext/pdo/tests/pdo_setfetchmode_atomic.phpt @@ -0,0 +1,73 @@ +--TEST-- +PDO Common: Failed setFetchMode must preserve the prior fetch mode and payload +--EXTENSIONS-- +pdo +--SKIPIF-- +<?php +$dir = getenv('REDIR_TEST_DIR'); +if ($dir === false) { + 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'; +$pdo = PDOTest::factory(); + +class FetchClass +{ + public int $i; + + public function __construct(public string $marker) + { + } +} + +$pdo->exec('CREATE TABLE pdo_setfetchmode_atomic (i INT)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_atomic (i) VALUES (1)'); +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_atomic'); +$row = new stdClass; +$stmt->setFetchMode(PDO::FETCH_INTO, $row); +try { + $stmt->setFetchMode(PDO::FETCH_CLASS); +} catch (Throwable $e) { + echo "into error: ", $e::class, "\n"; +} +$r = $stmt->fetch(); + +echo "into object preserved: "; +var_dump($r === $row); +echo "into value: ", $row->i, "\n"; + +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_atomic'); +$stmt->setFetchMode(PDO::FETCH_CLASS, FetchClass::class, ['kept']); +try { + $stmt->setFetchMode(PDO::FETCH_CLASS); +} catch (Throwable $e) { + echo "class error: ", $e::class, "\n"; +} +$r = $stmt->fetch(); + +echo "class preserved: ", $r::class, "\n"; +echo "constructor argument: ", $r->marker, "\n"; +echo "class value: ", $r->i, "\n"; +?> +--CLEAN-- +<?php +require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; +$pdo = PDOTest::factory(); +PDOTest::dropTableIfExists($pdo, 'pdo_setfetchmode_atomic'); +?> +--EXPECT-- +into error: ArgumentCountError +into object preserved: bool(true) +into value: 1 +class error: ArgumentCountError +class preserved: FetchClass +constructor argument: kept +class value: 1 diff --git a/ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt b/ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt new file mode 100644 index 000000000000..bc163366fa8d --- /dev/null +++ b/ext/pdo/tests/pdo_setfetchmode_reentrant_fetch.phpt @@ -0,0 +1,75 @@ +--TEST-- +PDO Common: setFetchMode autoload re-entry observes the prior complete mode +--EXTENSIONS-- +pdo +--SKIPIF-- +<?php +$dir = getenv('REDIR_TEST_DIR'); +if ($dir === false) { + 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'; +$pdo = PDOTest::factory(); + +class PriorFetchClass +{ + public int $i; + + public function __construct(public string $marker) + { + } +} + +$pdo->exec('CREATE TABLE pdo_setfetchmode_reentrant_fetch (i INT)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_fetch (i) VALUES (1)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_fetch (i) VALUES (2)'); + +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_fetch ORDER BY i'); +$into = new stdClass; +$stmt->setFetchMode(PDO::FETCH_INTO, $into); +$autoload = function (string $class) use ($stmt, $into): void { + $row = $stmt->fetch(); + echo "into re-entry preserved: "; + var_dump($row === $into); + eval("class $class { public int \$i; }"); +}; +spl_autoload_register($autoload); +$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredIntoClass'); +spl_autoload_unregister($autoload); +$row = $stmt->fetch(); +echo "into re-entry value: ", $into->i, "\n"; +echo "outer into class: ", $row::class, " ", $row->i, "\n"; + +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_fetch ORDER BY i'); +$stmt->setFetchMode(PDO::FETCH_CLASS, PriorFetchClass::class, ['kept']); +$autoload = function (string $class) use ($stmt): void { + $row = $stmt->fetch(); + echo "class re-entry: ", $row::class, " ", $row->marker, " ", $row->i, "\n"; + eval("class $class { public int \$i; }"); +}; +spl_autoload_register($autoload); +$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredFetchClass'); +spl_autoload_unregister($autoload); +$row = $stmt->fetch(); +echo "outer class: ", $row::class, " ", $row->i, "\n"; +?> +--CLEAN-- +<?php +require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; +$pdo = PDOTest::factory(); +PDOTest::dropTableIfExists($pdo, 'pdo_setfetchmode_reentrant_fetch'); +?> +--EXPECT-- +into re-entry preserved: bool(true) +into re-entry value: 1 +outer into class: ReenteredIntoClass 2 +class re-entry: PriorFetchClass kept 1 +outer class: ReenteredFetchClass 2 diff --git a/ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt b/ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt new file mode 100644 index 000000000000..7eb68e10136d --- /dev/null +++ b/ext/pdo/tests/pdo_setfetchmode_reentrant_setmode.phpt @@ -0,0 +1,53 @@ +--TEST-- +PDO Common: setFetchMode autoload re-entry may change the statement safely +--EXTENSIONS-- +pdo +--SKIPIF-- +<?php +$dir = getenv('REDIR_TEST_DIR'); +if ($dir === false) { + 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'; +$pdo = PDOTest::factory(); + +$pdo->exec('CREATE TABLE pdo_setfetchmode_reentrant_setmode (i INT)'); +$pdo->exec('INSERT INTO pdo_setfetchmode_reentrant_setmode (i) VALUES (1)'); +$stmt = $pdo->query('SELECT i FROM pdo_setfetchmode_reentrant_setmode'); +$first = new stdClass; +$second = new stdClass; +$stmt->setFetchMode(PDO::FETCH_INTO, $first); + +$autoload = function (string $class) use ($stmt, $second): void { + $stmt->setFetchMode(PDO::FETCH_INTO, $second); + eval("class $class { public int \$i; }"); +}; +spl_autoload_register($autoload); +$stmt->setFetchMode(PDO::FETCH_CLASS, 'ReenteredSetModeClass'); +spl_autoload_unregister($autoload); + +$row = $stmt->fetch(); +echo $row::class, " ", $row->i, "\n"; +echo "first changed: "; +var_dump(isset($first->i)); +echo "second changed: "; +var_dump(isset($second->i)); +?> +--CLEAN-- +<?php +require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; +$pdo = PDOTest::factory(); +PDOTest::dropTableIfExists($pdo, 'pdo_setfetchmode_reentrant_setmode'); +?> +--EXPECT-- +ReenteredSetModeClass 1 +first changed: bool(false) +second changed: bool(false)