Skip to content

Commit 4e55889

Browse files
Girgiasnielsdos
andauthored
ext/pdo: Refactor validation of fetch mode in PDO statement (#17699)
Co-authored-by: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
1 parent 1331a61 commit 4e55889

File tree

5 files changed

+170
-51
lines changed

5 files changed

+170
-51
lines changed

UPGRADING

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ PHP 8.5 UPGRADE NOTES
6161
array value reference assignment: $ctor_args = [&$valByRef]
6262
. Attempting to modify a PDOStatement during a call to PDO::fetch(),
6363
PDO::fetchObject(), PDO::fetchAll() will now throw an Error.
64+
. The value of the constants PDO::FETCH_GROUP, PDO::FETCH_UNIQUE,
65+
PDO::FETCH_CLASSTYPE, PDO::FETCH_PROPS_LATE, and PDO::FETCH_SERIALIZE
66+
has changed.
67+
. A ValueError is now thrown if PDO::FETCH_PROPS_LATE is used with a fetch
68+
mode different than PDO::FETCH_CLASS, consistent with other fetch flags.
69+
. A ValueError is now thrown if PDO::FETCH_INTO is used as a fetch mode in
70+
PDO::fetchAll(), similar to PDO::FETCH_LAZY.
6471

6572
- PDO_FIREBIRD:
6673
. A ValueError is now thrown when trying to set a cursor name that is too

ext/pdo/pdo_stmt.c

Lines changed: 61 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
693693
if (how == PDO_FETCH_COLUMN) {
694694
int colno = stmt->fetch.column;
695695

696-
if ((flags & PDO_FETCH_GROUP) && stmt->fetch.column == -1) {
696+
if ((flags & (PDO_FETCH_GROUP|PDO_FETCH_UNIQUE)) && stmt->fetch.column == -1) {
697697
colno = 1;
698698
}
699699

@@ -946,59 +946,81 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h
946946

947947

948948
// TODO Error on the following cases:
949-
// Using any fetch flag with PDO_FETCH_KEY_PAIR
950949
// Combining PDO_FETCH_UNIQUE and PDO_FETCH_GROUP
951-
// Using PDO_FETCH_UNIQUE or PDO_FETCH_GROUP outside of fetchAll()
952-
// Combining PDO_FETCH_PROPS_LATE with a fetch mode different than PDO_FETCH_CLASS
953-
// Improve error detection when combining PDO_FETCH_USE_DEFAULT with
954-
// Reject PDO_FETCH_INTO mode with fetch_all as no support
955950
// Reject $pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, value); bypass
956-
static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_arg_num, bool fetch_all) /* {{{ */
951+
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) /* {{{ */
957952
{
958-
int flags = mode & PDO_FETCH_FLAGS;
959-
960-
mode = mode & ~PDO_FETCH_FLAGS;
961-
962-
if (mode < 0 || mode >= PDO_FETCH__MAX) {
953+
/* Mode must be a positive */
954+
if (mode_and_flags < 0 || mode_and_flags >= PDO_FIRST_INVALID_FLAG) {
963955
zend_argument_value_error(mode_arg_num, "must be a bitmask of PDO::FETCH_* constants");
964-
return 0;
956+
return false;
965957
}
966958

959+
uint32_t flags = (zend_ulong)mode_and_flags & PDO_FETCH_FLAGS;
960+
enum pdo_fetch_type mode = (zend_ulong)mode_and_flags & ~PDO_FETCH_FLAGS;
961+
967962
if (mode == PDO_FETCH_USE_DEFAULT) {
968-
flags = stmt->default_fetch_type & PDO_FETCH_FLAGS;
969-
mode = stmt->default_fetch_type & ~PDO_FETCH_FLAGS;
963+
flags = default_mode_and_flags & PDO_FETCH_FLAGS;
964+
mode = default_mode_and_flags & ~PDO_FETCH_FLAGS;
970965
}
971966

972-
switch(mode) {
967+
/* Flags can only be used in limited circumstances */
968+
if (flags != 0) {
969+
bool has_class_flags = (flags & (PDO_FETCH_CLASSTYPE|PDO_FETCH_PROPS_LATE|PDO_FETCH_SERIALIZE)) != 0;
970+
if (has_class_flags && mode != PDO_FETCH_CLASS) {
971+
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");
972+
return false;
973+
}
974+
// TODO Prevent setting those flags together or not? This would affect PDO::setFetchMode()
975+
//bool has_grouping_flags = flags & (PDO_FETCH_GROUP|PDO_FETCH_UNIQUE);
976+
//if (has_grouping_flags && !fetch_all) {
977+
// zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_GROUP, or PDO::FETCH_UNIQUE fetch flags outside of PDOStatement::fetchAll()");
978+
// return false;
979+
//}
980+
if (flags & PDO_FETCH_SERIALIZE) {
981+
php_error_docref(NULL, E_DEPRECATED, "The PDO::FETCH_SERIALIZE mode is deprecated");
982+
if (UNEXPECTED(EG(exception))) {
983+
return false;
984+
}
985+
}
986+
}
987+
988+
switch (mode) {
973989
case PDO_FETCH_FUNC:
974990
if (!fetch_all) {
975-
zend_value_error("Can only use PDO::FETCH_FUNC in PDOStatement::fetchAll()");
976-
return 0;
991+
zend_argument_value_error(mode_arg_num, "PDO::FETCH_FUNC can only be used with PDOStatement::fetchAll()");
992+
return false;
977993
}
978-
return 1;
994+
return true;
979995

980996
case PDO_FETCH_LAZY:
981997
if (fetch_all) {
982-
zend_argument_value_error(mode_arg_num, "cannot be PDO::FETCH_LAZY in PDOStatement::fetchAll()");
983-
return 0;
984-
}
985-
ZEND_FALLTHROUGH;
986-
default:
987-
if ((flags & PDO_FETCH_SERIALIZE) == PDO_FETCH_SERIALIZE) {
988-
zend_argument_value_error(mode_arg_num, "must use PDO::FETCH_SERIALIZE with PDO::FETCH_CLASS");
989-
return 0;
998+
zend_argument_value_error(mode_arg_num, "PDO::FETCH_LAZY cannot be used with PDOStatement::fetchAll()");
999+
return false;
9901000
}
991-
if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) {
992-
zend_argument_value_error(mode_arg_num, "must use PDO::FETCH_CLASSTYPE with PDO::FETCH_CLASS");
993-
return 0;
1001+
return true;
1002+
1003+
case PDO_FETCH_INTO:
1004+
if (fetch_all) {
1005+
zend_argument_value_error(mode_arg_num, "PDO::FETCH_INTO cannot be used with PDOStatement::fetchAll()");
1006+
return false;
9941007
}
995-
ZEND_FALLTHROUGH;
1008+
return true;
9961009

1010+
case PDO_FETCH_ASSOC:
1011+
case PDO_FETCH_NUM:
1012+
case PDO_FETCH_BOTH:
1013+
case PDO_FETCH_OBJ:
1014+
case PDO_FETCH_BOUND:
1015+
case PDO_FETCH_COLUMN:
9971016
case PDO_FETCH_CLASS:
998-
if (flags & PDO_FETCH_SERIALIZE) {
999-
php_error_docref(NULL, E_DEPRECATED, "The PDO::FETCH_SERIALIZE mode is deprecated");
1000-
}
1001-
return 1;
1017+
case PDO_FETCH_NAMED:
1018+
case PDO_FETCH_KEY_PAIR:
1019+
return true;
1020+
1021+
default:
1022+
zend_argument_value_error(mode_arg_num, "must be a bitmask of PDO::FETCH_* constants");
1023+
return false;
10021024
}
10031025
}
10041026
/* }}} */
@@ -1020,7 +1042,7 @@ PHP_METHOD(PDOStatement, fetch)
10201042
PHP_STMT_GET_OBJ;
10211043
PDO_STMT_CLEAR_ERR();
10221044

1023-
if (!pdo_stmt_verify_mode(stmt, how, 1, false)) {
1045+
if (!pdo_verify_fetch_mode(stmt->default_fetch_type, how, 1, false)) {
10241046
RETURN_THROWS();
10251047
}
10261048

@@ -1140,7 +1162,7 @@ PHP_METHOD(PDOStatement, fetchAll)
11401162
ZEND_PARSE_PARAMETERS_END();
11411163

11421164
PHP_STMT_GET_OBJ;
1143-
if (!pdo_stmt_verify_mode(stmt, how, 1, true)) {
1165+
if (!pdo_verify_fetch_mode(stmt->default_fetch_type, how, 1, true)) {
11441166
RETURN_THROWS();
11451167
}
11461168

@@ -1215,7 +1237,7 @@ PHP_METHOD(PDOStatement, fetchAll)
12151237
}
12161238
stmt->fetch.column = Z_LVAL_P(arg2);
12171239
} else {
1218-
stmt->fetch.column = flags & PDO_FETCH_GROUP ? -1 : 0;
1240+
stmt->fetch.column = flags & (PDO_FETCH_GROUP|PDO_FETCH_UNIQUE) ? -1 : 0;
12191241
}
12201242
break;
12211243

@@ -1606,7 +1628,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a
16061628

16071629
flags = mode & PDO_FETCH_FLAGS;
16081630

1609-
if (!pdo_stmt_verify_mode(stmt, mode, mode_arg_num, false)) {
1631+
if (!pdo_verify_fetch_mode(stmt->default_fetch_type, mode, mode_arg_num, false)) {
16101632
return false;
16111633
}
16121634

ext/pdo/php_pdo_driver.h

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ enum pdo_param_type {
6363

6464
#define PDO_PARAM_TYPE(x) ((x) & ~PDO_PARAM_FLAGS)
6565

66+
/* Fetch mode is a bitmask of the fetch type (first 4 bits) with the fetch flags (bit 5 to 9)*/
6667
enum pdo_fetch_type {
6768
PDO_FETCH_USE_DEFAULT,
6869
PDO_FETCH_LAZY,
@@ -77,15 +78,17 @@ enum pdo_fetch_type {
7778
PDO_FETCH_FUNC, /* fetch into function and return its result */
7879
PDO_FETCH_NAMED, /* like PDO_FETCH_ASSOC, but can handle duplicate names */
7980
PDO_FETCH_KEY_PAIR, /* fetch into an array where the 1st column is a key and all subsequent columns are values */
80-
PDO_FETCH__MAX /* must be last */
8181
};
8282

83-
#define PDO_FETCH_FLAGS 0xFFFF0000 /* fetchAll() modes or'd to PDO_FETCH_XYZ */
84-
#define PDO_FETCH_GROUP 0x00010000 /* fetch into groups */
85-
#define PDO_FETCH_UNIQUE 0x00030000 /* fetch into groups assuming first col is unique */
86-
#define PDO_FETCH_CLASSTYPE 0x00040000 /* fetch class gets its class name from 1st column */
87-
#define PDO_FETCH_SERIALIZE 0x00080000 /* fetch class instances by calling serialize */
88-
#define PDO_FETCH_PROPS_LATE 0x00100000 /* fetch props after calling ctor */
83+
#define PDO_FETCH_FLAGS 0xFFFFFFF0 /* fetch flags mask */
84+
#define PDO_FETCH_GROUP (1u << 5u) /* fetch into groups */
85+
#define PDO_FETCH_UNIQUE (1u << 6u) /* fetch into groups assuming first col is unique */
86+
/* PDO_FETCH_CLASS only flags */
87+
#define PDO_FETCH_CLASSTYPE (1u << 7u) /* fetch class gets its class name from 1st column */
88+
#define PDO_FETCH_PROPS_LATE (1u << 8u) /* fetch props after calling ctor */
89+
#define PDO_FETCH_SERIALIZE (1u << 9u) /* DEPRECATED: fetch class instances by calling serialize */
90+
#define PDO_FIRST_INVALID_FLAG (1u << 10u)
91+
8992

9093
/* fetch orientation for scrollable cursors */
9194
enum pdo_fetch_orientation {

ext/pdo/tests/bug_38253.phpt

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var_dump($stmt->fetchAll());
3131
$pdo->setAttribute (PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_INTO);
3232
$stmt = $pdo->prepare ("SELECT * FROM test38253");
3333
$stmt->execute();
34-
var_dump($stmt->fetchAll());
34+
var_dump($stmt->fetch());
3535

3636
?>
3737
--CLEAN--
@@ -53,8 +53,7 @@ Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line
5353
array(0) {
5454
}
5555

56-
Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: No fetch-into object specified. in %s on line %d
56+
Warning: PDOStatement::fetch(): SQLSTATE[HY000]: General error: No fetch-into object specified. in %s on line %d
5757

58-
Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d
59-
array(0) {
60-
}
58+
Warning: PDOStatement::fetch(): SQLSTATE[HY000]: General error in %s on line %d
59+
bool(false)
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
--TEST--
2+
PDO Common: PDOStatement::setFetchMode() errors
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+
$db = PDOTest::factory();
17+
18+
$db->exec('CREATE TABLE pdo_set_fetch_mode_error(id int NOT NULL PRIMARY KEY, val VARCHAR(10), val2 VARCHAR(10))');
19+
$db->exec("INSERT INTO pdo_set_fetch_mode_error VALUES(1, 'A', 'AA')");
20+
$db->exec("INSERT INTO pdo_set_fetch_mode_error VALUES(2, 'B', 'BB')");
21+
$db->exec("INSERT INTO pdo_set_fetch_mode_error VALUES(3, 'C', 'CC')");
22+
23+
$stmt = $db->prepare('SELECT id, val, val2 from pdo_set_fetch_mode_error');
24+
25+
foreach (range(PDO::FETCH_KEY_PAIR, 16) as $mode) {
26+
try {
27+
echo "Mode: $mode\n";
28+
var_dump($stmt->setFetchMode($mode));
29+
} catch (\Throwable $e) {
30+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
31+
}
32+
}
33+
foreach (range(PDO::FETCH_KEY_PAIR, 16) as $mode) {
34+
try {
35+
echo "Mode: $mode\n";
36+
var_dump($stmt->setFetchMode($mode|PDO::FETCH_PROPS_LATE));
37+
} catch (\Throwable $e) {
38+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
39+
}
40+
}
41+
try {
42+
var_dump($stmt->setFetchMode(PDO::FETCH_KEY_PAIR|PDO::FETCH_PROPS_LATE));
43+
} catch (\Throwable $e) {
44+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
45+
}
46+
try {
47+
var_dump($stmt->setFetchMode(PDO::FETCH_KEY_PAIR|(PDO::FETCH_SERIALIZE*2)));
48+
} catch (\Throwable $e) {
49+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
50+
}
51+
try {
52+
var_dump($stmt->setFetchMode(PDO::FETCH_FUNC));
53+
} catch (\Throwable $e) {
54+
echo $e::class, ': ', $e->getMessage(), \PHP_EOL;
55+
}
56+
57+
58+
?>
59+
--CLEAN--
60+
<?php
61+
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
62+
$db = PDOTest::factory();
63+
PDOTest::dropTableIfExists($db, "pdo_set_fetch_mode_error");
64+
?>
65+
--EXPECT--
66+
Mode: 12
67+
bool(true)
68+
Mode: 13
69+
ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants
70+
Mode: 14
71+
ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants
72+
Mode: 15
73+
ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants
74+
Mode: 16
75+
bool(true)
76+
Mode: 12
77+
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
78+
Mode: 13
79+
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
80+
Mode: 14
81+
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
82+
Mode: 15
83+
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
84+
Mode: 16
85+
bool(true)
86+
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
87+
ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) must be a bitmask of PDO::FETCH_* constants
88+
ValueError: PDOStatement::setFetchMode(): Argument #1 ($mode) PDO::FETCH_FUNC can only be used with PDOStatement::fetchAll()

0 commit comments

Comments
 (0)