Skip to content

ext/pdo: Default Fetch mode handling refactoring. #17694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

ext/pdo: Default Fetch mode handling refactoring. #17694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d917497
ext/pdo: Add const modifiers to pdo_get_TYPE_param() functions
Girgias Feb 4, 2025
8c9fc83
ext/pdo: Refactor validation of fetch mode value
Girgias Feb 4, 2025
d6da66c
ext/pdo: Move default fetch flags into bitfield
Girgias Feb 4, 2025
8184dbe
ext/pdo: Pass argument number to pdo_dbh_attribute_set()
Girgias Feb 4, 2025
9edd83b
ext/pdo: Revome useless ext/standard header include
Girgias Feb 4, 2025
e3d0526
ext/pdo: Prevent setting bogus default fetch modes from PDO::setAttri…
Girgias Feb 4, 2025
5327fa9
ext/pdo: Remove now useless checks and convert them into assertions
Girgias Feb 4, 2025
5a7ae0c
ext/pdo: Move default_fetch_mode into PDO bitfield
Girgias Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions UPGRADING
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ 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.

- PDO_FIREBIRD:
. A ValueError is now thrown when trying to set a cursor name that is too
Expand Down
59 changes: 29 additions & 30 deletions ext/pdo/pdo_dbh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

#include "php.h"
#include "php_ini.h"
#include "ext/standard/info.h"
#include "php_pdo.h"
#include "php_pdo_driver.h"
#include "php_pdo_int.h"
Expand All @@ -36,7 +35,7 @@
#include "zend_observer.h"
#include "zend_extensions.h"

static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value);
static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value, uint32_t value_arg_num);

void pdo_throw_exception(unsigned int driver_errcode, char *driver_errmsg, pdo_error_type *pdo_error)
{
Expand Down Expand Up @@ -513,7 +512,7 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
ZVAL_DEREF(attr_value);

/* TODO: Should the constructor fail when the attribute cannot be set? */
pdo_dbh_attribute_set(dbh, long_key, attr_value);
pdo_dbh_attribute_set(dbh, long_key, attr_value, 3);
} ZEND_HASH_FOREACH_END();
}

Expand Down Expand Up @@ -777,7 +776,7 @@ PHP_METHOD(PDO, inTransaction)
}
/* }}} */

PDO_API bool pdo_get_long_param(zend_long *lval, zval *value)
PDO_API bool pdo_get_long_param(zend_long *lval, const zval *value)
{
switch (Z_TYPE_P(value)) {
case IS_LONG:
Expand All @@ -795,7 +794,7 @@ PDO_API bool pdo_get_long_param(zend_long *lval, zval *value)
return false;
}
}
PDO_API bool pdo_get_bool_param(bool *bval, zval *value)
PDO_API bool pdo_get_bool_param(bool *bval, const zval *value)
{
switch (Z_TYPE_P(value)) {
case IS_TRUE:
Expand All @@ -815,7 +814,7 @@ PDO_API bool pdo_get_bool_param(bool *bval, zval *value)
}

/* Return false on failure, true otherwise */
static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* {{{ */
static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value, uint32_t value_arg_num) /* {{{ */
{
zend_long lval;
bool bval;
Expand All @@ -832,7 +831,7 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
dbh->error_mode = lval;
return true;
default:
zend_value_error("Error mode must be one of the PDO::ERRMODE_* constants");
zend_argument_value_error(value_arg_num, "Error mode must be one of the PDO::ERRMODE_* constants");
return false;
}
return false;
Expand All @@ -848,7 +847,7 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
dbh->desired_case = lval;
return true;
default:
zend_value_error("Case folding mode must be one of the PDO::CASE_* constants");
zend_argument_value_error(value_arg_num, "Case folding mode must be one of the PDO::CASE_* constants");
return false;
}
return false;
Expand All @@ -862,22 +861,22 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
return true;

case PDO_ATTR_DEFAULT_FETCH_MODE:
if (Z_TYPE_P(value) == IS_ARRAY) {
zval *tmp;
if ((tmp = zend_hash_index_find(Z_ARRVAL_P(value), 0)) != NULL && Z_TYPE_P(tmp) == IS_LONG) {
if (Z_LVAL_P(tmp) == PDO_FETCH_INTO || Z_LVAL_P(tmp) == PDO_FETCH_CLASS) {
zend_value_error("PDO::FETCH_INTO and PDO::FETCH_CLASS cannot be set as the default fetch mode");
return false;
}
}
lval = zval_get_long(value);
} else {
if (!pdo_get_long_param(&lval, value)) {
return false;
}
if (!pdo_get_long_param(&lval, value)) {
return false;
}
if (!pdo_verify_fetch_mode(PDO_FETCH_USE_DEFAULT, lval, value_arg_num, false)) {
return false;
}
if (UNEXPECTED(
lval == PDO_FETCH_USE_DEFAULT
|| lval == PDO_FETCH_INTO
|| lval == PDO_FETCH_FUNC
)) {
zend_argument_value_error(value_arg_num, "cannot set default fetch mode to PDO::FETCH_USE_DEFAULT, PDO::FETCH_INTO, PDO::FETCH_FUNC");
return false;
}
if (lval == PDO_FETCH_USE_DEFAULT) {
zend_value_error("Fetch mode must be a bitmask of PDO::FETCH_* constants");
if (UNEXPECTED((lval & PDO_FETCH_CLASS) && (lval & PDO_FETCH_CLASSTYPE) == 0)) {
zend_argument_value_error(value_arg_num, "cannot set default fetch mode to PDO::FETCH_CLASS without PDO::FETCH_CLASSTYPE");
return false;
}
dbh->default_fetch_type = lval;
Expand Down Expand Up @@ -907,25 +906,25 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
return false;
}
if (Z_TYPE_P(value) != IS_ARRAY) {
zend_type_error("PDO::ATTR_STATEMENT_CLASS value must be of type array, %s given",
zend_argument_type_error(value_arg_num, "PDO::ATTR_STATEMENT_CLASS value must be of type array, %s given",
zend_zval_value_name(value));
return false;
}
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL) {
zend_value_error("PDO::ATTR_STATEMENT_CLASS value must be an array with the format "
zend_argument_value_error(value_arg_num, "PDO::ATTR_STATEMENT_CLASS value must be an array with the format "
"array(classname, constructor_args)");
return false;
}
if (Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL) {
zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be a valid class");
zend_argument_type_error(value_arg_num, "PDO::ATTR_STATEMENT_CLASS class must be a valid class");
return false;
}
if (!instanceof_function(pce, pdo_dbstmt_ce)) {
zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement");
zend_argument_type_error(value_arg_num, "PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement");
return false;
}
if (pce->constructor && !(pce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) {
zend_type_error("User-supplied statement class cannot have a public constructor");
zend_argument_type_error(value_arg_num, "User-supplied statement class cannot have a public constructor");
return false;
}
dbh->def_stmt_ce = pce;
Expand All @@ -935,7 +934,7 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
}
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 1)) != NULL) {
if (Z_TYPE_P(item) != IS_ARRAY) {
zend_type_error("PDO::ATTR_STATEMENT_CLASS constructor_args must be of type ?array, %s given",
zend_argument_type_error(value_arg_num, "PDO::ATTR_STATEMENT_CLASS constructor_args must be of type ?array, %s given",
zend_zval_value_name(value));
return false;
}
Expand Down Expand Up @@ -981,7 +980,7 @@ PHP_METHOD(PDO, setAttribute)
PDO_DBH_CLEAR_ERR();
PDO_CONSTRUCT_CHECK;

RETURN_BOOL(pdo_dbh_attribute_set(dbh, attr, value));
RETURN_BOOL(pdo_dbh_attribute_set(dbh, attr, value, 2));
}
/* }}} */

Expand Down
133 changes: 65 additions & 68 deletions ext/pdo/pdo_stmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -763,13 +763,6 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
}
zval_ptr_dtor(&ce_name_from_column);
} else {
/* This can happen if the fetch flags are set via PDO::setAttribute()
* $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_CLASS);
* See ext/pdo/tests/bug_38253.phpt */
if (UNEXPECTED(ce == NULL)) {
pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch class specified");
goto in_fetch_error;
}
ctor_arguments = stmt->fetch.cls.ctor_args;
}
ZEND_ASSERT(ce != NULL);
Expand All @@ -794,14 +787,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
break;

case PDO_FETCH_INTO:
/* This can happen if the fetch flags are set via PDO::setAttribute()
* $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_INTO);
* See ext/pdo/tests/bug_38253.phpt */
if (stmt->fetch.into == NULL) {
pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch-into object specified.");
goto in_fetch_error;
}

ZEND_ASSERT(stmt->fetch.into != NULL);
ZVAL_OBJ_COPY(return_value, stmt->fetch.into);

/* We want the behaviour of fetching into an object to be called from the global scope rather
Expand All @@ -810,13 +796,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
break;

case PDO_FETCH_FUNC:
/* This can happen if the fetch flags are set via PDO::setAttribute()
* $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_FUNC);
* See ext/pdo/tests/bug_38253.phpt */
if (UNEXPECTED(!ZEND_FCC_INITIALIZED(stmt->fetch.func.fcc))) {
pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch function specified");
goto in_fetch_error;
}
ZEND_ASSERT(ZEND_FCC_INITIALIZED(stmt->fetch.func.fcc));
/* There will be at most stmt->column_count parameters.
* However, if we fetch a group key we will have over allocated. */
fetch_function_params = safe_emalloc(sizeof(zval), stmt->column_count, 0);
Expand Down Expand Up @@ -946,59 +926,80 @@ 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) /* {{{ */
PDO_API 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;
}

/* 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) {
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;
}
}
/* }}} */
Expand All @@ -1020,7 +1021,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();
}

Expand Down Expand Up @@ -1140,7 +1141,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();
}

Expand Down Expand Up @@ -1215,7 +1216,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;

Expand Down Expand Up @@ -1576,12 +1577,8 @@ void pdo_stmt_free_default_fetch_mode(pdo_stmt_t *stmt)
{
enum pdo_fetch_type default_fetch_mode = stmt->default_fetch_type & ~PDO_FETCH_FLAGS;
if (default_fetch_mode == PDO_FETCH_INTO) {
/* This can happen if the fetch flags are set via PDO::setAttribute()
* $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_INTO);
* See ext/pdo/tests/bug_38253.phpt */
if (EXPECTED(stmt->fetch.into != NULL)) {
OBJ_RELEASE(stmt->fetch.into);
}
ZEND_ASSERT(stmt->fetch.into != NULL);
OBJ_RELEASE(stmt->fetch.into);
} else if (default_fetch_mode == PDO_FETCH_CLASS) {
if (stmt->fetch.cls.ctor_args != NULL) {
zend_array_release(stmt->fetch.cls.ctor_args);
Expand All @@ -1606,7 +1603,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;
}

Expand Down
Loading
Loading