From afeb60c5888a2925ffe0023a4b39e9908fb8782a Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 4 Feb 2025 13:37:19 +0000 Subject: [PATCH 1/4] ext/pdo: Refactor validation of fetch mode value This affects the value of the fetch mode flags --- ext/pdo/pdo_stmt.c | 100 +++++++++++++++++++++-------------- ext/pdo/php_pdo_driver.h | 17 +++--- ext/pdo/tests/bug_38253.phpt | 9 ++-- 3 files changed, 75 insertions(+), 51 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index b2b4984ef8177..b0ea907cc6df9 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -693,7 +693,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h if (how == PDO_FETCH_COLUMN) { int colno = stmt->fetch.column; - if ((flags & PDO_FETCH_GROUP) && stmt->fetch.column == -1) { + if ((flags & (PDO_FETCH_GROUP|PDO_FETCH_UNIQUE)) && stmt->fetch.column == -1) { colno = 1; } @@ -946,59 +946,81 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h // TODO Error on the following cases: -// Using any fetch flag with PDO_FETCH_KEY_PAIR // Combining PDO_FETCH_UNIQUE and PDO_FETCH_GROUP -// Using PDO_FETCH_UNIQUE or PDO_FETCH_GROUP outside of fetchAll() -// Combining PDO_FETCH_PROPS_LATE with a fetch mode different than PDO_FETCH_CLASS -// Improve error detection when combining PDO_FETCH_USE_DEFAULT with -// Reject PDO_FETCH_INTO mode with fetch_all as no support // Reject $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, value); bypass -static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_arg_num, bool fetch_all) /* {{{ */ +static bool pdo_verify_fetch_mode(uint32_t default_mode_and_flags, zend_long mode_and_flags, uint32_t mode_arg_num, bool fetch_all) /* {{{ */ { - int flags = mode & PDO_FETCH_FLAGS; - - mode = mode & ~PDO_FETCH_FLAGS; - - if (mode < 0 || mode >= PDO_FETCH__MAX) { + /* Mode must be a positive */ + if (mode_and_flags < 0 || mode_and_flags >= PDO_FIRST_INVALID_FLAG) { zend_argument_value_error(mode_arg_num, "must be a bitmask of PDO::FETCH_* constants"); - return 0; + return false; } + uint32_t flags = (zend_ulong)mode_and_flags & PDO_FETCH_FLAGS; + enum pdo_fetch_type mode = (zend_ulong)mode_and_flags & ~PDO_FETCH_FLAGS; + if (mode == PDO_FETCH_USE_DEFAULT) { - flags = stmt->default_fetch_type & PDO_FETCH_FLAGS; - mode = stmt->default_fetch_type & ~PDO_FETCH_FLAGS; + flags = default_mode_and_flags & PDO_FETCH_FLAGS; + mode = default_mode_and_flags & ~PDO_FETCH_FLAGS; } - switch(mode) { + /* Flags can only be used in limited circumstances */ + if (flags != 0) { + bool has_class_flags = (flags & (PDO_FETCH_CLASSTYPE|PDO_FETCH_PROPS_LATE|PDO_FETCH_SERIALIZE)) != 0; + if (has_class_flags && mode != PDO_FETCH_CLASS) { + zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode different than PDO::FETCH_CLASS"); + return false; + } + // TODO Prevent setting those flags together or not? This would affect PDO::setFetchMode() + //bool has_grouping_flags = flags & (PDO_FETCH_GROUP|PDO_FETCH_UNIQUE); + //if (has_grouping_flags && !fetch_all) { + // zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_GROUP, or PDO::FETCH_UNIQUE fetch flags outside of PDOStatemnt::fetchAll()"); + // return false; + //} + if (flags & PDO_FETCH_SERIALIZE) { + php_error_docref(NULL, E_DEPRECATED, "The PDO::FETCH_SERIALIZE mode is deprecated"); + if (UNEXPECTED(EG(exception))) { + return false; + } + } + } + + switch (mode) { case PDO_FETCH_FUNC: if (!fetch_all) { - zend_value_error("Can only use PDO::FETCH_FUNC in PDOStatement::fetchAll()"); - return 0; + zend_argument_value_error(mode_arg_num, "PDO::FETCH_FUNC can only be used with PDOStatement::fetchAll()"); + return false; } - return 1; + return true; case PDO_FETCH_LAZY: if (fetch_all) { - zend_argument_value_error(mode_arg_num, "cannot be PDO::FETCH_LAZY in PDOStatement::fetchAll()"); - return 0; - } - ZEND_FALLTHROUGH; - default: - if ((flags & PDO_FETCH_SERIALIZE) == PDO_FETCH_SERIALIZE) { - zend_argument_value_error(mode_arg_num, "must use PDO::FETCH_SERIALIZE with PDO::FETCH_CLASS"); - return 0; + zend_argument_value_error(mode_arg_num, "PDO::FETCH_LAZY cannot be used with PDOStatement::fetchAll()"); + return false; } - if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) { - zend_argument_value_error(mode_arg_num, "must use PDO::FETCH_CLASSTYPE with PDO::FETCH_CLASS"); - return 0; + return true; + + case PDO_FETCH_INTO: + if (fetch_all) { + zend_argument_value_error(mode_arg_num, "PDO::FETCH_INTO cannot be used with PDOStatement::fetchAll()"); + return false; } - ZEND_FALLTHROUGH; + return true; + case PDO_FETCH_ASSOC: + case PDO_FETCH_NUM: + case PDO_FETCH_BOTH: + case PDO_FETCH_OBJ: + case PDO_FETCH_BOUND: + case PDO_FETCH_COLUMN: case PDO_FETCH_CLASS: - if (flags & PDO_FETCH_SERIALIZE) { - php_error_docref(NULL, E_DEPRECATED, "The PDO::FETCH_SERIALIZE mode is deprecated"); - } - return 1; + case PDO_FETCH_NAMED: + case PDO_FETCH_KEY_PAIR: + return true; + + default: + zend_argument_value_error(mode_arg_num, "must be a bitmask of PDO::FETCH_* constants"); + return false; } } /* }}} */ @@ -1020,7 +1042,7 @@ PHP_METHOD(PDOStatement, fetch) PHP_STMT_GET_OBJ; PDO_STMT_CLEAR_ERR(); - if (!pdo_stmt_verify_mode(stmt, how, 1, false)) { + if (!pdo_verify_fetch_mode(stmt->default_fetch_type, how, 1, false)) { RETURN_THROWS(); } @@ -1140,7 +1162,7 @@ PHP_METHOD(PDOStatement, fetchAll) ZEND_PARSE_PARAMETERS_END(); PHP_STMT_GET_OBJ; - if (!pdo_stmt_verify_mode(stmt, how, 1, true)) { + if (!pdo_verify_fetch_mode(stmt->default_fetch_type, how, 1, true)) { RETURN_THROWS(); } @@ -1215,7 +1237,7 @@ PHP_METHOD(PDOStatement, fetchAll) } stmt->fetch.column = Z_LVAL_P(arg2); } else { - stmt->fetch.column = flags & PDO_FETCH_GROUP ? -1 : 0; + stmt->fetch.column = flags & (PDO_FETCH_GROUP|PDO_FETCH_UNIQUE) ? -1 : 0; } break; @@ -1606,7 +1628,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a flags = mode & PDO_FETCH_FLAGS; - if (!pdo_stmt_verify_mode(stmt, mode, mode_arg_num, false)) { + if (!pdo_verify_fetch_mode(stmt->default_fetch_type, mode, mode_arg_num, false)) { return false; } diff --git a/ext/pdo/php_pdo_driver.h b/ext/pdo/php_pdo_driver.h index 98bcacee690e2..1bbdb6f0eceac 100644 --- a/ext/pdo/php_pdo_driver.h +++ b/ext/pdo/php_pdo_driver.h @@ -63,6 +63,7 @@ enum pdo_param_type { #define PDO_PARAM_TYPE(x) ((x) & ~PDO_PARAM_FLAGS) +/* Fetch mode is a bitmask of the fetch type (first 4 bits) with the fetch flags (bit 5 to 9)*/ enum pdo_fetch_type { PDO_FETCH_USE_DEFAULT, PDO_FETCH_LAZY, @@ -77,15 +78,17 @@ enum pdo_fetch_type { PDO_FETCH_FUNC, /* fetch into function and return its result */ PDO_FETCH_NAMED, /* like PDO_FETCH_ASSOC, but can handle duplicate names */ PDO_FETCH_KEY_PAIR, /* fetch into an array where the 1st column is a key and all subsequent columns are values */ - PDO_FETCH__MAX /* must be last */ }; -#define PDO_FETCH_FLAGS 0xFFFF0000 /* fetchAll() modes or'd to PDO_FETCH_XYZ */ -#define PDO_FETCH_GROUP 0x00010000 /* fetch into groups */ -#define PDO_FETCH_UNIQUE 0x00030000 /* fetch into groups assuming first col is unique */ -#define PDO_FETCH_CLASSTYPE 0x00040000 /* fetch class gets its class name from 1st column */ -#define PDO_FETCH_SERIALIZE 0x00080000 /* fetch class instances by calling serialize */ -#define PDO_FETCH_PROPS_LATE 0x00100000 /* fetch props after calling ctor */ +#define PDO_FETCH_FLAGS 0xFFFFFFF0 /* fetch flags mask */ +#define PDO_FETCH_GROUP (1u << 5u) /* fetch into groups */ +#define PDO_FETCH_UNIQUE (1u << 6u) /* fetch into groups assuming first col is unique */ +/* PDO_FETCH_CLASS only flags */ +#define PDO_FETCH_CLASSTYPE (1u << 7u) /* fetch class gets its class name from 1st column */ +#define PDO_FETCH_PROPS_LATE (1u << 8u) /* fetch props after calling ctor */ +#define PDO_FETCH_SERIALIZE (1u << 9u) /* DEPRECATED: fetch class instances by calling serialize */ +#define PDO_FIRST_INVALID_FLAG (1u << 10u) + /* fetch orientation for scrollable cursors */ enum pdo_fetch_orientation { diff --git a/ext/pdo/tests/bug_38253.phpt b/ext/pdo/tests/bug_38253.phpt index 91d94d31aeef8..06a9736ed85bb 100644 --- a/ext/pdo/tests/bug_38253.phpt +++ b/ext/pdo/tests/bug_38253.phpt @@ -31,7 +31,7 @@ var_dump($stmt->fetchAll()); $pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_INTO); $stmt = $pdo->prepare ("SELECT * FROM test38253"); $stmt->execute(); -var_dump($stmt->fetchAll()); +var_dump($stmt->fetch()); ?> --CLEAN-- @@ -53,8 +53,7 @@ Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line array(0) { } -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: No fetch-into object specified. in %s on line %d +Warning: PDOStatement::fetch(): SQLSTATE[HY000]: General error: No fetch-into object specified. in %s on line %d -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -array(0) { -} +Warning: PDOStatement::fetch(): SQLSTATE[HY000]: General error in %s on line %d +bool(false) From 58da42a000c33468dcaac90e400b9d3f59d18838 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sat, 8 Feb 2025 15:59:23 +0000 Subject: [PATCH 2/4] [skip ci] Wording Co-authored-by: Niels Dossche <7771979+nielsdos@users.noreply.github.com> --- ext/pdo/pdo_stmt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index b0ea907cc6df9..040c99ef42023 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -968,13 +968,13 @@ static bool pdo_verify_fetch_mode(uint32_t default_mode_and_flags, zend_long mod if (flags != 0) { bool has_class_flags = (flags & (PDO_FETCH_CLASSTYPE|PDO_FETCH_PROPS_LATE|PDO_FETCH_SERIALIZE)) != 0; if (has_class_flags && mode != PDO_FETCH_CLASS) { - zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode different than PDO::FETCH_CLASS"); + zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode other than PDO::FETCH_CLASS"); return false; } // TODO Prevent setting those flags together or not? This would affect PDO::setFetchMode() //bool has_grouping_flags = flags & (PDO_FETCH_GROUP|PDO_FETCH_UNIQUE); //if (has_grouping_flags && !fetch_all) { - // zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_GROUP, or PDO::FETCH_UNIQUE fetch flags outside of PDOStatemnt::fetchAll()"); + // zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_GROUP, or PDO::FETCH_UNIQUE fetch flags outside of PDOStatement::fetchAll()"); // return false; //} if (flags & PDO_FETCH_SERIALIZE) { From 0cf1e6be00958694b2e4039921f591f6c0721ad7 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sat, 8 Feb 2025 22:24:28 +0000 Subject: [PATCH 3/4] Add test --- ext/pdo/tests/pdo_set_fetch_mode_errors.phpt | 88 ++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 ext/pdo/tests/pdo_set_fetch_mode_errors.phpt diff --git a/ext/pdo/tests/pdo_set_fetch_mode_errors.phpt b/ext/pdo/tests/pdo_set_fetch_mode_errors.phpt new file mode 100644 index 0000000000000..bd8621fecc193 --- /dev/null +++ b/ext/pdo/tests/pdo_set_fetch_mode_errors.phpt @@ -0,0 +1,88 @@ +--TEST-- +PDO Common: PDOStatement::setFetchMode() errors +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE pdo_set_fetch_mode_error(id int NOT NULL PRIMARY KEY, val VARCHAR(10), val2 VARCHAR(10))'); +$db->exec("INSERT INTO pdo_set_fetch_mode_error VALUES(1, 'A', 'AA')"); +$db->exec("INSERT INTO pdo_set_fetch_mode_error VALUES(2, 'B', 'BB')"); +$db->exec("INSERT INTO pdo_set_fetch_mode_error VALUES(3, 'C', 'CC')"); + +$stmt = $db->prepare('SELECT id, val, val2 from pdo_set_fetch_mode_error'); + +foreach (range(PDO::FETCH_KEY_PAIR, 16) as $mode) { + try { + echo "Mode: $mode\n"; + var_dump($stmt->setFetchMode($mode)); + } catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), \PHP_EOL; + } +} +foreach (range(PDO::FETCH_KEY_PAIR, 16) as $mode) { + try { + echo "Mode: $mode\n"; + var_dump($stmt->setFetchMode($mode|PDO::FETCH_PROPS_LATE)); + } catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), \PHP_EOL; + } +} +try { + var_dump($stmt->setFetchMode(PDO::FETCH_KEY_PAIR|PDO::FETCH_PROPS_LATE)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), \PHP_EOL; +} +try { + var_dump($stmt->setFetchMode(PDO::FETCH_KEY_PAIR|(PDO::FETCH_SERIALIZE*2))); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), \PHP_EOL; +} +try { + var_dump($stmt->setFetchMode(PDO::FETCH_FUNC)); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), \PHP_EOL; +} + + +?> +--CLEAN-- + +--EXPECT-- +Mode: 12 +bool(true) +Mode: 13 +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants +Mode: 14 +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants +Mode: 15 +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants +Mode: 16 +bool(true) +Mode: 12 +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode other than PDO::FETCH_CLASS +Mode: 13 +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode other than PDO::FETCH_CLASS +Mode: 14 +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode other than PDO::FETCH_CLASS +Mode: 15 +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode other than PDO::FETCH_CLASS +Mode: 16 +bool(true) +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) cannot use PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, or PDO::FETCH_SERIALIZE fetch flags with a fetch mode other than PDO::FETCH_CLASS +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants +ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) PDO::FETCH_FUNC can only be used with PDOStatement::fetchAll() From ef824c47e1a50144609fee0c454de7ae10954710 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sun, 9 Feb 2025 00:33:57 +0000 Subject: [PATCH 4/4] [skip ci] UPGRADING --- UPGRADING | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/UPGRADING b/UPGRADING index 7149579d4732a..7e0e5fdad06ee 100644 --- a/UPGRADING +++ b/UPGRADING @@ -61,6 +61,13 @@ PHP 8.5 UPGRADE NOTES array value reference assignment: $ctor_args = [&$valByRef] . Attempting to modify a PDOStatement during a call to PDO::fetch(), PDO::fetchObject(), PDO::fetchAll() will now throw an Error. + . The value of the constants PDO::FETCH_GROUP, PDO::FETCH_UNIQUE, + PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, and PDO::FETCH_SERIALIZE + has changed. + . A ValueError is now thrown if PDO::FETCH_PROPS_LATE is used with a fetch + mode different than PDO::FETCH_CLASS, consistent with other fetch flags. + . A ValueError is now thrown if PDO::FETCH_INTO is used as a fetch mode in + PDO::fetchAll(), similar to PDO::FETCH_LAZY. - PDO_FIREBIRD: . A ValueError is now thrown when trying to set a cursor name that is too