PDO cleanup & improvement

UPGRADING.INTERNALS

@@ -68,3 +68,9 @@ PHP 8.1 INTERNALS UPGRADE NOTES
       of char* for the optional sequence/table name.
     - The php_pdo_str_tolower_dup() PDO_API has been removed use zend_str_tolower_dup()
       or zend_string_tolower_ex().
+    - A new PDO_API pdo_get_long_param(zend_long *lval, zval *value) has been
+      introduced to fetch integer values for driver attributes, it will throw
+      a type error and return false in case of failure.
+    - A new PDO_API pdo_get_bool_param(zend_long *bool, zval *value) has been
+      introduced to fetch boolean values for driver attributes, it will throw
+      a type error and return false in case of failure.

ext/pdo/pdo_dbh.c

@@ -678,22 +678,54 @@ PHP_METHOD(PDO, inTransaction)
 }
 /* }}} */
 
-/* TODO: Make distinction between numeric and non-numeric strings */
-#define PDO_LONG_PARAM_CHECK \
-	if (Z_TYPE_P(value) != IS_LONG && Z_TYPE_P(value) != IS_STRING && Z_TYPE_P(value) != IS_FALSE && Z_TYPE_P(value) != IS_TRUE) { \
-		zend_type_error("Attribute value must be of type int for selected attribute, %s given", zend_zval_type_name(value)); \
-		return false; \
-	} \
+PDO_API bool pdo_get_long_param(zend_long *lval, zval *value)
+{
+	switch (Z_TYPE_P(value)) {
+		case IS_LONG:
+		case IS_TRUE:
+		case IS_FALSE:
+			*lval = zval_get_long(value);
+			return true;
+		case IS_STRING:
+			if (IS_LONG == is_numeric_str_function(Z_STR_P(value), lval, NULL)) {
+				return true;
+			}
+			/* fallthrough */
+		default:
+			zend_type_error("Attribute value must be of type int for selected attribute, %s given", zend_zval_type_name(value));
+			return false;
+	}
+}
+PDO_API bool pdo_get_bool_param(bool *bval, zval *value)
+{
+	switch (Z_TYPE_P(value)) {
+		case IS_TRUE:
+			*bval = true;
+			return true;
+		case IS_FALSE:
+			*bval = false;
+			return true;
+		case IS_LONG:
+			*bval = zval_is_true(value);
+			return true;
+		case IS_STRING: /* TODO Should string be allowed? */
+		default:
+			zend_type_error("Attribute value must be of type bool for selected attribute, %s given", zend_zval_type_name(value));
+			return false;
+	}
+}
 
 /* Return false on failure, true otherwise */
 static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* {{{ */
 {
 	zend_long lval;
+	bool bval;
 
 	switch (attr) {
 		case PDO_ATTR_ERRMODE:
-			PDO_LONG_PARAM_CHECK;
-			lval = zval_get_long(value);
+			if (!pdo_get_long_param(&lval, value)) {
+				return false;
+			}
 			switch (lval) {
 				case PDO_ERRMODE_SILENT:
 				case PDO_ERRMODE_WARNING:
@@ -707,8 +739,9 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
 			return false;
 
 		case PDO_ATTR_CASE:
-			PDO_LONG_PARAM_CHECK;
-			lval = zval_get_long(value);
+			if (!pdo_get_long_param(&lval, value)) {
+				return false;
+			}
 			switch (lval) {
 				case PDO_CASE_NATURAL:
 				case PDO_CASE_UPPER:
@@ -722,8 +755,11 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
 			return false;
 
 		case PDO_ATTR_ORACLE_NULLS:
-			PDO_LONG_PARAM_CHECK;
-			dbh->oracle_nulls = zval_get_long(value);
+			if (!pdo_get_long_param(&lval, value)) {
+				return false;
+			}
+			/* TODO Check for valid value (NULL_NATURAL, NULL_EMPTY_STRING, NULL_TO_STRING)? */
+			dbh->oracle_nulls = lval;
 			return true;
 
 		case PDO_ATTR_DEFAULT_FETCH_MODE:
@@ -735,10 +771,12 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
 						return false;
 					}
 				}
+				lval = zval_get_long(value);
 			} else {
-				PDO_LONG_PARAM_CHECK;
+				if (!pdo_get_long_param(&lval, value)) {
+					return false;
+				}
 			}
-			lval = zval_get_long(value);
 			if (lval == PDO_FETCH_USE_DEFAULT) {
 				zend_value_error("Fetch mode must be a bitmask of PDO::FETCH_* constants");
 				return false;
@@ -747,8 +785,10 @@ static bool pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /
 			return true;
 
 		case PDO_ATTR_STRINGIFY_FETCHES:
-			PDO_LONG_PARAM_CHECK;
-			dbh->stringify = zval_get_long(value) ? 1 : 0;
+			if (!pdo_get_bool_param(&bval, value)) {
+				return false;
+			}
+			dbh->stringify = bval;
 			return true;
 
 		case PDO_ATTR_STATEMENT_CLASS: {

ext/pdo/php_pdo_driver.h

@@ -681,5 +681,9 @@ PDO_API void php_pdo_dbh_delref(pdo_dbh_t *dbh);
 PDO_API void php_pdo_free_statement(pdo_stmt_t *stmt);
 PDO_API void php_pdo_stmt_set_column_count(pdo_stmt_t *stmt, int new_count);
 
+/* Normalization for fetching long param for driver attributes */
+PDO_API bool pdo_get_long_param(zend_long *lval, zval *value);
+PDO_API bool pdo_get_bool_param(bool *bval, zval *value);
+
 PDO_API void pdo_throw_exception(unsigned int driver_errcode, char *driver_errmsg, pdo_error_type *pdo_error);
 #endif /* PHP_PDO_DRIVER_H */

ext/pdo/tests/bug_44159.phpt

@@ -42,6 +42,6 @@ foreach ($attrs as $attr) {
 TypeError: PDO::ATTR_STATEMENT_CLASS value must be of type array, null given
 TypeError: PDO::ATTR_STATEMENT_CLASS value must be of type array, int given
 TypeError: PDO::ATTR_STATEMENT_CLASS value must be of type array, string given
-TypeError: Attribute value must be of type int for selected attribute, null given
-bool(true)
+TypeError: Attribute value must be of type bool for selected attribute, null given
 bool(true)
+TypeError: Attribute value must be of type bool for selected attribute, string given

ext/pdo_dblib/dblib_driver.c

@@ -273,22 +273,39 @@ zend_string *dblib_handle_last_id(pdo_dbh_t *dbh, const zend_string *name)
 static bool dblib_set_attr(pdo_dbh_t *dbh, zend_long attr, zval *val)
 {
 	pdo_dblib_db_handle *H = (pdo_dblib_db_handle *)dbh->driver_data;
+	zend_long lval;
+	bool bval;
 
 	switch(attr) {
 		case PDO_ATTR_DEFAULT_STR_PARAM:
-			H->assume_national_character_set_strings = zval_get_long(val) == PDO_PARAM_STR_NATL ? 1 : 0;
+			if (!pdo_get_long_param(&lval, val)) {
+				return false;
+			}
+			H->assume_national_character_set_strings = lval == PDO_PARAM_STR_NATL ? 1 : 0;
 			return true;
 		case PDO_ATTR_TIMEOUT:
 		case PDO_DBLIB_ATTR_QUERY_TIMEOUT:
-			return SUCCEED == dbsettime(zval_get_long(val));
+			if (!pdo_get_long_param(&lval, val)) {
+				return false;
+			}
+			return SUCCEED == dbsettime(lval);
 		case PDO_DBLIB_ATTR_STRINGIFY_UNIQUEIDENTIFIER:
-			H->stringify_uniqueidentifier = zval_get_long(val);
+			if (!pdo_get_long_param(&lval, val)) {
+				return false;
+			}
+			H->stringify_uniqueidentifier = lval;
 			return true;
 		case PDO_DBLIB_ATTR_SKIP_EMPTY_ROWSETS:
-			H->skip_empty_rowsets = zval_is_true(val);
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
+			H->skip_empty_rowsets = bval;
 			return true;
 		case PDO_DBLIB_ATTR_DATETIME_CONVERT:
-			H->datetime_convert = zval_get_long(val);
+			if (!pdo_get_long_param(&lval, val)) {
+				return false;
+			}
+			H->datetime_convert = lval;
 			return true;
 		default:
 			return false;

ext/pdo_firebird/firebird_driver.c

@@ -820,11 +820,14 @@ static int firebird_alloc_prepare_stmt(pdo_dbh_t *dbh, const zend_string *sql,
 static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val) /* {{{ */
 {
 	pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data;
+	bool bval;
 
 	switch (attr) {
 		case PDO_ATTR_AUTOCOMMIT:
 			{
-				bool bval = zval_get_long(val)? 1 : 0;
+				if (!pdo_get_bool_param(&bval, val)) {
+					return false;
+				}
 
 				/* ignore if the new value equals the old one */
 				if (dbh->auto_commit ^ bval) {
@@ -848,7 +851,10 @@ static bool firebird_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *
 			return true;
 
 		case PDO_ATTR_FETCH_TABLE_NAMES:
-			H->fetch_table_names = zval_get_long(val)? 1 : 0;
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
+			H->fetch_table_names = bval;
 			return true;
 
 		case PDO_FB_ATTR_DATE_FORMAT:

ext/pdo_mysql/mysql_driver.c

@@ -402,13 +402,17 @@ static inline int mysql_handle_autocommit(pdo_dbh_t *dbh)
 /* {{{ pdo_mysql_set_attribute */
 static bool pdo_mysql_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val)
 {
-	zend_long lval = zval_get_long(val);
-	bool bval = lval ? 1 : 0;
+	zend_long lval;
+	bool bval;
 	PDO_DBG_ENTER("pdo_mysql_set_attribute");
 	PDO_DBG_INF_FMT("dbh=%p", dbh);
 	PDO_DBG_INF_FMT("attr=%l", attr);
+
 	switch (attr) {
 		case PDO_ATTR_AUTOCOMMIT:
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
 			/* ignore if the new value equals the old one */
 			if (dbh->auto_commit ^ bval) {
 				dbh->auto_commit = bval;
@@ -419,26 +423,41 @@ static bool pdo_mysql_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val)
 			PDO_DBG_RETURN(true);
 
 		case PDO_ATTR_DEFAULT_STR_PARAM:
+			if (!pdo_get_long_param(&lval, val)) {
+				PDO_DBG_RETURN(false);
+			}
 			((pdo_mysql_db_handle *)dbh->driver_data)->assume_national_character_set_strings = lval == PDO_PARAM_STR_NATL;
 			PDO_DBG_RETURN(true);
 
 		case PDO_MYSQL_ATTR_USE_BUFFERED_QUERY:
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
 			/* ignore if the new value equals the old one */
 			((pdo_mysql_db_handle *)dbh->driver_data)->buffered = bval;
 			PDO_DBG_RETURN(true);
 
 		case PDO_MYSQL_ATTR_DIRECT_QUERY:
 		case PDO_ATTR_EMULATE_PREPARES:
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
 			/* ignore if the new value equals the old one */
 			((pdo_mysql_db_handle *)dbh->driver_data)->emulate_prepare = bval;
 			PDO_DBG_RETURN(true);
 
 		case PDO_ATTR_FETCH_TABLE_NAMES:
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
 			((pdo_mysql_db_handle *)dbh->driver_data)->fetch_table_names = bval;
 			PDO_DBG_RETURN(true);
 
 #ifndef PDO_USE_MYSQLND
 		case PDO_MYSQL_ATTR_MAX_BUFFER_SIZE:
+			if (!pdo_get_long_param(&lval, val)) {
+				PDO_DBG_RETURN(false);
+			}
 			if (lval < 0) {
 				/* TODO: Johannes, can we throw a warning here? */
  				((pdo_mysql_db_handle *)dbh->driver_data)->max_buffer_size = 1024*1024;

ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt

@@ -25,7 +25,6 @@ error_reporting=E_ALL
         echo get_class($e), ': ', $e->getMessage(), \PHP_EOL;
     }
     try {
-        /* This currently passes */
         $db->setAttribute(PDO::ATTR_ERRMODE, 'pdo');
     } catch (\Error $e) {
         echo get_class($e), ': ', $e->getMessage(), \PHP_EOL;
@@ -160,6 +159,7 @@ error_reporting=E_ALL
 --EXPECTF--
 TypeError: Attribute value must be of type int for selected attribute, array given
 TypeError: Attribute value must be of type int for selected attribute, stdClass given
+TypeError: Attribute value must be of type int for selected attribute, string given
 ValueError: Error mode must be one of the PDO::ERRMODE_* constants
 
 Warning: PDO::query(): SQLSTATE[42000]: Syntax error or access violation: %d You have an error in your SQL syntax; check the manual that corresponds to your %s server version for the right syntax to use near '%s' at line %d in %s on line %d

ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt

@@ -23,7 +23,6 @@ MySQLPDOTest::skip();
         echo $e->getMessage(), \PHP_EOL;
     }
     try {
-        /* Currently passes... */
         $db->setAttribute(PDO::ATTR_ORACLE_NULLS, 'pdo');
     } catch (\TypeError $e) {
         echo $e->getMessage(), \PHP_EOL;
@@ -72,6 +71,7 @@ MySQLPDOTest::skip();
 --EXPECTF--
 Attribute value must be of type int for selected attribute, array given
 Attribute value must be of type int for selected attribute, stdClass given
+Attribute value must be of type int for selected attribute, string given
 array(1) {
   [0]=>
   array(6) {

ext/pdo_oci/oci_driver.c

@@ -425,12 +425,17 @@ static bool oci_handle_rollback(pdo_dbh_t *dbh) /* {{{ */
 
 static bool oci_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val) /* {{{ */
 {
-	zend_long lval = zval_get_long(val);
+	zend_long lval;
 	pdo_oci_db_handle *H = (pdo_oci_db_handle *)dbh->driver_data;
 
 	switch (attr) {
 		case PDO_ATTR_AUTOCOMMIT:
 		{
+			bool bval;
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
+
 			if (dbh->in_txn) {
 				/* Assume they want to commit whatever is outstanding */
 				H->last_err = OCITransCommit(H->svc, H->err, 0);
@@ -442,11 +447,15 @@ static bool oci_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val)
 				dbh->in_txn = false;
 			}
 
-			dbh->auto_commit = (unsigned int)lval? 1 : 0;
+			dbh->auto_commit = (unsigned int) bval;
 			return true;
 		}
 		case PDO_ATTR_PREFETCH:
 		{
+			if (!pdo_get_long_param(&lval, val)) {
+				return false;
+			}
+
 			H->prefetch = pdo_oci_sanitize_prefetch(lval);
 			return true;
 		}
@@ -537,6 +546,9 @@ static bool oci_handle_set_attribute(pdo_dbh_t *dbh, zend_long attr, zval *val)
 		case PDO_OCI_ATTR_CALL_TIMEOUT:
 		{
 #if (OCI_MAJOR_VERSION >= 18)
+			if (!pdo_get_long_param(&lval, val)) {
+				return false;
+			}
 			ub4 timeout = (ub4) lval;
 
 			H->last_err = OCIAttrSet(H->svc, OCI_HTYPE_SVCCTX,

ext/pdo_odbc/odbc_driver.c

@@ -333,9 +333,14 @@ static bool odbc_handle_rollback(pdo_dbh_t *dbh)
 static bool odbc_handle_set_attr(pdo_dbh_t *dbh, zend_long attr, zval *val)
 {
 	pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data;
+	bool bval;
+
 	switch (attr) {
 		case PDO_ODBC_ATTR_ASSUME_UTF8:
-			H->assume_utf8 = zval_is_true(val);
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
+			H->assume_utf8 = bval;
 			return true;
 		default:
 			strcpy(H->einfo.last_err_msg, "Unknown Attribute");

ext/pdo_pgsql/pgsql_driver.c

@@ -1154,14 +1154,20 @@ static const zend_function_entry *pdo_pgsql_get_driver_methods(pdo_dbh_t *dbh, i
 
 static bool pdo_pgsql_set_attr(pdo_dbh_t *dbh, zend_long attr, zval *val)
 {
-	bool bval = zval_get_long(val)? 1 : 0;
+	bool bval;
 	pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data;
 
 	switch (attr) {
 		case PDO_ATTR_EMULATE_PREPARES:
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
 			H->emulate_prepares = bval;
 			return true;
 		case PDO_PGSQL_ATTR_DISABLE_PREPARES:
+			if (!pdo_get_bool_param(&bval, val)) {
+				return false;
+			}
 			H->disable_prepares = bval;
 			return true;
 		default: