ext/pdo: Refactor validation of fetch mode in PDO statement

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

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 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 PDOStatement::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;
 	}
 

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 {

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)

ext/pdo/tests/pdo_set_fetch_mode_errors.phpt

@@ -0,0 +1,88 @@
+--TEST--
+PDO Common: PDOStatement::setFetchMode() errors
+--EXTENSIONS--
+pdo
+--SKIPIF--
+<?php
+$dir = getenv('REDIR_TEST_DIR');
+if (false == $dir) 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';
+$db = PDOTest::factory();
+
+$db->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--
+<?php
+require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+PDOTest::dropTableIfExists($db, "pdo_set_fetch_mode_error");
+?>
+--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()