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 3 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
3 commits
Select commit Hold shift + click to select a range
771267e
ext/pdo: Move default fetch flags into bitfield
Girgias Feb 4, 2025
0154578
ext/pdo: Prevent setting bogus default fetch modes from PDO::setAttri…
Girgias Feb 4, 2025
deaa2d4
ext/pdo: Remove now useless checks and convert them into assertions
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
30 changes: 15 additions & 15 deletions ext/pdo/pdo_dbh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,22 +857,22 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value, u
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_argument_value_error(value_arg_num, "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_argument_value_error(value_arg_num, "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
35 changes: 5 additions & 30 deletions ext/pdo/pdo_stmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,13 +764,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 @@ -795,14 +788,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 @@ -811,13 +797,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 @@ -948,8 +928,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h

// TODO Error on the following cases:
// Combining PDO_FETCH_UNIQUE and PDO_FETCH_GROUP
// Reject $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, value); bypass
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) /* {{{ */
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) /* {{{ */
{
/* Mode must be a positive */
if (mode_and_flags < 0 || mode_and_flags >= PDO_FIRST_INVALID_FLAG) {
Expand Down Expand Up @@ -1599,12 +1578,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 Down
10 changes: 6 additions & 4 deletions ext/pdo/php_pdo_driver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,10 @@ struct _pdo_stmt_t {
* bindParam() for its prepared statements, if false, PDO should
* emulate prepare and bind on its behalf */
uint16_t supports_placeholders:2;
uint16_t reserved: 12;

/* defaults for fetches */
uint16_t default_fetch_type:9;
uint16_t reserved:3;

/* keep track of bound input parameters. Some drivers support
* input/output parameters, but you can't rely on that working */
Expand All @@ -595,9 +598,6 @@ struct _pdo_stmt_t {
* */
int32_t column_count;

/* defaults for fetches */
enum pdo_fetch_type default_fetch_type;

union {
int column;
struct {
Expand Down Expand Up @@ -704,6 +704,8 @@ PDO_API void php_pdo_internal_construct_driver(INTERNAL_FUNCTION_PARAMETERS, zen
PDO_API bool pdo_get_long_param(zend_long *lval, const zval *value);
PDO_API bool pdo_get_bool_param(bool *bval, const zval *value);

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);

PDO_API void pdo_throw_exception(unsigned int driver_errcode, char *driver_errmsg, pdo_error_type *pdo_error);

/* When a GC cycle is collected, it's possible that the database object is destroyed prior to destroying
Expand Down
57 changes: 28 additions & 29 deletions ext/pdo/tests/bug_38253.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,32 @@ $pdo = PDOTest::factory();
$pdo->exec ("create table test38253 (id integer primary key, n varchar(255))");
$pdo->exec ("INSERT INTO test38253 (id, n) VALUES (1, 'hi')");

$pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_CLASS);
$stmt = $pdo->prepare ("SELECT * FROM test38253");
$stmt->execute();
var_dump($stmt->fetchAll());
try {
$pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_CLASS);
$stmt = $pdo->prepare ("SELECT * FROM test38253");
$stmt->execute();
var_dump($stmt->fetchAll());
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
}

$pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_FUNC);
$stmt = $pdo->prepare ("SELECT * FROM test38253");
$stmt->execute();
var_dump($stmt->fetchAll());
try {
$pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_FUNC);
$stmt = $pdo->prepare ("SELECT * FROM test38253");
$stmt->execute();
var_dump($stmt->fetchAll());
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
}

$pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_INTO);
$stmt = $pdo->prepare ("SELECT * FROM test38253");
$stmt->execute();
var_dump($stmt->fetch());
try {
$pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_INTO);
$stmt = $pdo->prepare ("SELECT * FROM test38253");
$stmt->execute();
var_dump($stmt->fetch());
} catch (\Throwable $e) {
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
}

?>
--CLEAN--
Expand All @@ -40,20 +52,7 @@ require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
$db = PDOTest::factory();
PDOTest::dropTableIfExists($db, "test38253");
?>
--EXPECTF--
Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: No fetch class specified in %s on line %d

Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d
array(0) {
}

Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: No fetch function 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: No fetch-into object specified. in %s on line %d

Warning: PDOStatement::fetch(): SQLSTATE[HY000]: General error in %s on line %d
bool(false)
--EXPECT--
ValueError: PDO::setAttribute(): Argument #2 ($value) cannot set default fetch mode to PDO::FETCH_CLASS without PDO::FETCH_CLASSTYPE
ValueError: PDO::setAttribute(): Argument #2 ($value) PDO::FETCH_FUNC can only be used with PDOStatement::fetchAll()
ValueError: PDO::setAttribute(): Argument #2 ($value) cannot set default fetch mode to PDO::FETCH_USE_DEFAULT, PDO::FETCH_INTO, PDO::FETCH_FUNC
Loading