From 2435c87d90a5542f8a1c5d402b0676384801269c Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 00:41:21 +0100 Subject: [PATCH 01/50] Fix a logical bug in PDO pdo_stmt_verify_mode() never returns FAILURE (aka -1) and thus this path was never hit --- ext/pdo/pdo_stmt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 8ed0a77636e8b..da1496f6f6349 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1110,7 +1110,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h } /* }}} */ -static int pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, int fetch_all) /* {{{ */ +static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, int fetch_all) /* {{{ */ { int flags = mode & PDO_FETCH_FLAGS; From e280e5e47e6a5ec5aff7a7911b8209bc8a1505f0 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 00:40:26 +0100 Subject: [PATCH 02/50] Column index must be >= 0 (just 1 case) and todo comment. --- ext/pdo/pdo_stmt.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index da1496f6f6349..5ecf4e5894f6f 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -457,7 +457,13 @@ static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, int *typ int caller_frees = 0; int type, new_type; - if (colno < 0 || colno >= stmt->column_count) { + if (colno < 0) { + zend_value_error("Column index must be greater than or equal to 0"); + ZVAL_NULL(dest); + return; + } + + if (colno >= stmt->column_count) { pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Invalid column index"); ZVAL_FALSE(dest); @@ -666,6 +672,7 @@ static int do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */ fcc->called_scope = ce; return 1; } else if (!Z_ISUNDEF(stmt->fetch.cls.ctor_args)) { + /* Promote to Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "user-supplied class does not have a constructor, use NULL for the ctor_params parameter, or simply omit it"); return 0; } else { From 96770fa28189a3b22be0391687a7c98d4511b247 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 02:42:37 +0100 Subject: [PATCH 03/50] Refactor PDO setFetchMode --- ext/pdo/pdo_dbh.c | 11 +-- ext/pdo/pdo_dbh.stub.php | 2 +- ext/pdo/pdo_dbh_arginfo.h | 5 +- ext/pdo/pdo_stmt.c | 162 ++++++++++++++++++----------------- ext/pdo/pdo_stmt.stub.php | 2 +- ext/pdo/pdo_stmt_arginfo.h | 5 +- ext/pdo/php_pdo_int.h | 4 +- ext/pdo/tests/bug_44173.phpt | 61 +++++++------ ext/pdo/tests/pdo_038.phpt | 9 +- 9 files changed, 142 insertions(+), 119 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 780de94707b1e..a2d01f33ef3ff 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -1055,12 +1055,13 @@ PHP_METHOD(PDO, query) size_t statement_len; zend_long fetch_mode; zend_bool fetch_mode_is_null = 1; - zval *args = NULL; - uint32_t num_args = 0; + zval *arg1 = NULL; + HashTable *constructor_args = NULL; pdo_dbh_object_t *dbh_obj = Z_PDO_OBJECT_P(ZEND_THIS); pdo_dbh_t *dbh = dbh_obj->inner; - if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "s|l!*", &statement, &statement_len, &fetch_mode, &fetch_mode_is_null, &args, &num_args)) { + if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "s|l!z!h!", &statement, &statement_len, + &fetch_mode, &fetch_mode_is_null, &arg1, &constructor_args)) { RETURN_THROWS(); } @@ -1090,8 +1091,8 @@ PHP_METHOD(PDO, query) if (dbh->methods->preparer(dbh, statement, statement_len, stmt, NULL)) { PDO_STMT_CLEAR_ERR(); - if (fetch_mode_is_null || SUCCESS == pdo_stmt_setup_fetch_mode(stmt, fetch_mode, args, num_args)) { - + if (fetch_mode_is_null || pdo_stmt_setup_fetch_mode(stmt, ZEND_NUM_ARGS(), + fetch_mode, 2, arg1, 3, constructor_args, 4)) { /* now execute the statement */ PDO_STMT_CLEAR_ERR(); if (stmt->methods->executer(stmt)) { diff --git a/ext/pdo/pdo_dbh.stub.php b/ext/pdo/pdo_dbh.stub.php index 2348abf048d86..ba33baf59e8ce 100644 --- a/ext/pdo/pdo_dbh.stub.php +++ b/ext/pdo/pdo_dbh.stub.php @@ -37,7 +37,7 @@ public function lastInsertId(?string $name = null) {} public function prepare(string $statement, array $driver_options = []) {} /** @return PDOStatement|false */ - public function query(string $statement, ?int $fetch_mode = null, mixed ...$fetch_mode_args) {} + public function query(string $statement, ?int $fetch_mode = null, mixed $arg1 = null, ?array $constructorArguments = []) {} /** @return string|false */ public function quote(string $string, int $parameter_type = PDO::PARAM_STR) {} diff --git a/ext/pdo/pdo_dbh_arginfo.h b/ext/pdo/pdo_dbh_arginfo.h index a3ab1994ed67d..179821bf20583 100644 --- a/ext/pdo/pdo_dbh_arginfo.h +++ b/ext/pdo/pdo_dbh_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 0c7acc78768ad1fb77a09a24ebd4d4e660b350c9 */ + * Stub hash: 1374ebd53913af744634fcbd745f0182d87fff1a */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDO___construct, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, dsn, IS_STRING, 0) @@ -41,7 +41,8 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDO_query, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, statement, IS_STRING, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, fetch_mode, IS_LONG, 1, "null") - ZEND_ARG_VARIADIC_TYPE_INFO(0, fetch_mode_args, IS_MIXED, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, arg1, IS_MIXED, 0, "null") + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, constructorArguments, IS_ARRAY, 1, "[]") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDO_quote, 0, 0, 1) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 5ecf4e5894f6f..88121217be560 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1117,14 +1117,14 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h } /* }}} */ -static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, int fetch_all) /* {{{ */ +static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, 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) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "invalid fetch mode"); + zend_argument_value_error(mode_arg_num, "must be a bitmask of PDO::FETCH_* constants"); return 0; } @@ -1157,7 +1157,8 @@ static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, int fetch_all return 0; } if (mode >= PDO_FETCH__MAX) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "invalid fetch mode"); + /* TODO Better error message? */ + zend_argument_value_error(mode_arg_num, "must be a bitmask of PDO::FETCH_* constants"); return 0; } /* no break; */ @@ -1185,7 +1186,7 @@ PHP_METHOD(PDOStatement, fetch) PHP_STMT_GET_OBJ; PDO_STMT_CLEAR_ERR(); - if (!pdo_stmt_verify_mode(stmt, how, 0)) { + if (!pdo_stmt_verify_mode(stmt, how, 1, false)) { RETURN_FALSE; } @@ -1199,7 +1200,6 @@ PHP_METHOD(PDOStatement, fetch) /* {{{ Fetches the next row and returns it as an object. */ PHP_METHOD(PDOStatement, fetchObject) { - zend_long how = PDO_FETCH_CLASS; zend_long ori = PDO_FETCH_ORI_NEXT; zend_long off = 0; zend_class_entry *ce = NULL; @@ -1216,7 +1216,7 @@ PHP_METHOD(PDOStatement, fetchObject) PHP_STMT_GET_OBJ; PDO_STMT_CLEAR_ERR(); - if (!pdo_stmt_verify_mode(stmt, how, 0)) { + if (!pdo_stmt_verify_mode(stmt, PDO_FETCH_CLASS, 0, false)) { RETURN_FALSE; } @@ -1239,7 +1239,7 @@ PHP_METHOD(PDOStatement, fetchObject) stmt->fetch.cls.ce = zend_standard_class_def; } - if (!do_fetch(stmt, return_value, how, ori, off, 0)) { + if (!do_fetch(stmt, return_value, PDO_FETCH_CLASS, ori, off, 0)) { PDO_HANDLE_STMT_ERR(); RETVAL_FALSE; } @@ -1291,7 +1291,7 @@ PHP_METHOD(PDOStatement, fetchAll) ZEND_PARSE_PARAMETERS_END(); PHP_STMT_GET_OBJ; - if (!pdo_stmt_verify_mode(stmt, how, 1)) { + if (!pdo_stmt_verify_mode(stmt, how, 1, true)) { RETURN_FALSE; } @@ -1723,13 +1723,11 @@ PHP_METHOD(PDOStatement, getColumnMeta) /* {{{ Changes the default fetch mode for subsequent fetches (params have different meaning for different fetch modes) */ -int pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, zval *args, uint32_t num_args) +bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_long fetch_mode, + uint32_t mode_arg_num, zval *arg1, uint32_t arg1_arg_num, HashTable *constructor_args, + uint32_t constructor_arg_num) { int flags = 0; - zend_class_entry *cep; - int retval; - - do_fetch_opt_finish(stmt, 1); switch (stmt->default_fetch_type) { case PDO_FETCH_INTO: @@ -1744,15 +1742,14 @@ int pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, zval *args, uint stmt->default_fetch_type = PDO_FETCH_BOTH; - flags = mode & PDO_FETCH_FLAGS; + flags = fetch_mode & PDO_FETCH_FLAGS; - if (!pdo_stmt_verify_mode(stmt, mode, 0)) { + if (!pdo_stmt_verify_mode(stmt, fetch_mode, mode_arg_num, false)) { PDO_STMT_CLEAR_ERR(); - return FAILURE; + return false; } - retval = FAILURE; - switch (mode & ~PDO_FETCH_FLAGS) { + switch (fetch_mode & ~PDO_FETCH_FLAGS) { case PDO_FETCH_USE_DEFAULT: case PDO_FETCH_LAZY: case PDO_FETCH_ASSOC: @@ -1762,90 +1759,88 @@ int pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, zval *args, uint case PDO_FETCH_BOUND: case PDO_FETCH_NAMED: case PDO_FETCH_KEY_PAIR: - if (num_args != 0) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode doesn't allow any extra arguments"); - } else { - retval = SUCCESS; + if (total_num_args != mode_arg_num) { + zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + ZSTR_VAL(get_active_function_or_method_name()), mode_arg_num, total_num_args); + return false; } break; case PDO_FETCH_COLUMN: - if (num_args != 1) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode requires the colno argument"); - } else if (Z_TYPE(args[0]) != IS_LONG) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "colno must be an integer"); - } else { - stmt->fetch.column = Z_LVAL(args[0]); - retval = SUCCESS; + if (total_num_args != arg1_arg_num) { + zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + ZSTR_VAL(get_active_function_or_method_name()), arg1_arg_num, total_num_args); + return false; + } + if (Z_TYPE_P(arg1) != IS_LONG) { + zend_argument_type_error(arg1_arg_num, "must be int, %s given", zend_zval_type_name(arg1)); + return false; } + if (Z_LVAL_P(arg1) < 0) { + zend_argument_value_error(arg1_arg_num, "must be greater than or equal to 0"); + return false; + } + stmt->fetch.column = Z_LVAL_P(arg1); break; case PDO_FETCH_CLASS: + /* Undef constructor arguments */ + ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); /* Gets its class name from 1st column */ if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) { - if (num_args != 0) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode doesn't allow any extra arguments"); - } else { - stmt->fetch.cls.ce = NULL; - retval = SUCCESS; + if (arg1 || constructor_args) { + zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + ZSTR_VAL(get_active_function_or_method_name()), mode_arg_num, total_num_args); + return false; } + stmt->fetch.cls.ce = NULL; } else { - if (num_args < 1) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode requires the classname argument"); - } else if (num_args > 2) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "too many arguments"); - } else if (Z_TYPE(args[0]) != IS_STRING) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "classname must be a string"); - } else { - cep = zend_lookup_class(Z_STR(args[0])); - if (cep) { - retval = SUCCESS; - stmt->fetch.cls.ce = cep; - } - } - } + zend_class_entry *cep; - if (SUCCESS == retval) { - ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); - if (num_args == 2) { - if (Z_TYPE(args[1]) != IS_NULL && Z_TYPE(args[1]) != IS_ARRAY) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "ctor_args must be either NULL or an array"); - retval = FAILURE; - } else if (Z_TYPE(args[1]) == IS_ARRAY && zend_hash_num_elements(Z_ARRVAL(args[1]))) { - ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(Z_ARRVAL(args[1]))); - } + if (!arg1 || total_num_args > constructor_arg_num) { + zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + ZSTR_VAL(get_active_function_or_method_name()), constructor_arg_num, total_num_args); + return false; } - - if (SUCCESS == retval) { - do_fetch_class_prepare(stmt); + if (Z_TYPE_P(arg1) != IS_STRING) { + zend_argument_type_error(arg1_arg_num, "must be string, %s given", zend_zval_type_name(arg1)); + return false; + } + cep = zend_lookup_class(Z_STR_P(arg1)); + if (!cep) { + zend_argument_type_error(arg1_arg_num, "must be a valid class"); + return false; + } + stmt->fetch.cls.ce = cep; + if (constructor_args && zend_hash_num_elements(constructor_args)) { + ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(constructor_args)); } } + do_fetch_class_prepare(stmt); break; case PDO_FETCH_INTO: - if (num_args != 1) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode requires the object parameter"); - } else if (Z_TYPE(args[0]) != IS_OBJECT) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "object must be an object"); - } else { - retval = SUCCESS; + if (total_num_args != arg1_arg_num) { + zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + ZSTR_VAL(get_active_function_or_method_name()), arg1_arg_num, total_num_args); + return false; } - - if (SUCCESS == retval) { - ZVAL_COPY(&stmt->fetch.into, &args[0]); + if (Z_TYPE_P(arg1) != IS_OBJECT) { + zend_argument_type_error(arg1_arg_num, "must be object, %s given", zend_zval_type_name(arg1)); + return false; } + ZVAL_COPY(&stmt->fetch.into, arg1); break; - default: - pdo_raise_impl_error(stmt->dbh, stmt, "22003", "Invalid fetch mode specified"); + zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants"); + return false; } - if (SUCCESS == retval) { - stmt->default_fetch_type = mode; - } + stmt->default_fetch_type = fetch_mode; + /* TODO Is this still needed? */ /* * PDO error (if any) has already been raised at this point. * @@ -1855,21 +1850,30 @@ int pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, zval *args, uint */ PDO_STMT_CLEAR_ERR(); - return retval; + return true; } PHP_METHOD(PDOStatement, setFetchMode) { zend_long fetch_mode; - zval *args = NULL; - uint32_t num_args = 0; + zval *arg1 = NULL; + HashTable *constructor_args = NULL; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "l*", &fetch_mode, &args, &num_args) == FAILURE) { + /* Support null for constructor arguments for BC */ + if (zend_parse_parameters(ZEND_NUM_ARGS(), "l|z!h!", &fetch_mode, &arg1, &constructor_args) == FAILURE) { RETURN_THROWS(); } PHP_STMT_GET_OBJ; - RETVAL_BOOL(pdo_stmt_setup_fetch_mode(stmt, fetch_mode, args, num_args) == SUCCESS); + + do_fetch_opt_finish(stmt, 1); + + if (!pdo_stmt_setup_fetch_mode(stmt, ZEND_NUM_ARGS(), fetch_mode, 1, arg1, 2, constructor_args, 3)) { + RETURN_THROWS(); + } + + // TODO Void return? + RETURN_TRUE; } /* }}} */ diff --git a/ext/pdo/pdo_stmt.stub.php b/ext/pdo/pdo_stmt.stub.php index b280f73ffdb3d..137586079e41d 100644 --- a/ext/pdo/pdo_stmt.stub.php +++ b/ext/pdo/pdo_stmt.stub.php @@ -59,7 +59,7 @@ public function rowCount() {} public function setAttribute(int $attribute, mixed $value) {} /** @return bool */ - public function setFetchMode(int $mode, mixed ...$params) {} + public function setFetchMode(int $mode, mixed $arg1 = null, ?array $constructorArguments = []) {} public function getIterator(): Iterator {} } diff --git a/ext/pdo/pdo_stmt_arginfo.h b/ext/pdo/pdo_stmt_arginfo.h index 116449ad3aefe..ba95ede9c354c 100644 --- a/ext/pdo/pdo_stmt_arginfo.h +++ b/ext/pdo/pdo_stmt_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: a35e66ccff5e569f07ae8372e661e005943dfbc7 */ + * Stub hash: afff84f6e878c3d8837bf53a4ce3617b9c7fb386 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_bindColumn, 0, 0, 2) ZEND_ARG_TYPE_MASK(0, column, MAY_BE_STRING|MAY_BE_LONG, NULL) @@ -77,7 +77,8 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_setFetchMode, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, mode, IS_LONG, 0) - ZEND_ARG_VARIADIC_TYPE_INFO(0, params, IS_MIXED, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, arg1, IS_MIXED, 0, "null") + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, constructorArguments, IS_ARRAY, 1, "[]") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class_PDOStatement_getIterator, 0, 0, Iterator, 0) diff --git a/ext/pdo/php_pdo_int.h b/ext/pdo/php_pdo_int.h index 7fe3bfc85023f..4f37d26358556 100644 --- a/ext/pdo/php_pdo_int.h +++ b/ext/pdo/php_pdo_int.h @@ -40,7 +40,9 @@ void pdo_dbstmt_free_storage(zend_object *std); zend_object_iterator *pdo_stmt_iter_get(zend_class_entry *ce, zval *object, int by_ref); extern zend_object_handlers pdo_dbstmt_object_handlers; int pdo_stmt_describe_columns(pdo_stmt_t *stmt); -int pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long fetch_mode, zval *args, uint32_t num_args); +bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_long mode, + uint32_t mode_arg_num, zval *arg1, uint32_t arg1_arg_num, HashTable *constructor_args, + uint32_t constructor_arg_num); extern zend_object *pdo_row_new(zend_class_entry *ce); extern const zend_function_entry pdo_row_functions[]; diff --git a/ext/pdo/tests/bug_44173.phpt b/ext/pdo/tests/bug_44173.phpt index df98f332fe188..16b3c1fbd8dc1 100644 --- a/ext/pdo/tests/bug_44173.phpt +++ b/ext/pdo/tests/bug_44173.phpt @@ -19,8 +19,12 @@ $db->exec("INSERT INTO test VALUES (1)"); // Bug entry [2] -- 1 is PDO::FETCH_LAZY -$stmt = $db->query("SELECT * FROM test", PDO::FETCH_LAZY, 0, 0); -var_dump($stmt); +try { + $stmt = $db->query("SELECT * FROM test", PDO::FETCH_LAZY, 0, []); + var_dump($stmt); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} // Bug entry [3] @@ -31,39 +35,46 @@ try { } // Bug entry [4] -$stmt = $db->query("SELECT * FROM test", PDO::FETCH_CLASS, 0, 0, 0); -var_dump($stmt); +try { + $stmt = $db->query("SELECT * FROM test", PDO::FETCH_CLASS, 0, 0, 0); + var_dump($stmt); +} catch (\ArgumentCountError $e) { + echo $e->getMessage(), \PHP_EOL; +} // Bug entry [5] -$stmt = $db->query("SELECT * FROM test", PDO::FETCH_INTO); -var_dump($stmt); +try { + $stmt = $db->query("SELECT * FROM test", PDO::FETCH_INTO); + var_dump($stmt); +} catch (\ArgumentCountError $e) { + echo $e->getMessage(), \PHP_EOL; +} // Bug entry [6] -$stmt = $db->query("SELECT * FROM test", PDO::FETCH_COLUMN); -var_dump($stmt); +try { + $stmt = $db->query("SELECT * FROM test", PDO::FETCH_COLUMN); + var_dump($stmt); +} catch (\ArgumentCountError $e) { + echo $e->getMessage(), \PHP_EOL; +} // Bug entry [7] -$stmt = $db->query("SELECT * FROM test", PDO::FETCH_CLASS); -var_dump($stmt); +try { + $stmt = $db->query("SELECT * FROM test", PDO::FETCH_CLASS); + var_dump($stmt); +} catch (\ArgumentCountError $e) { + echo $e->getMessage(), \PHP_EOL; +} ?> ---EXPECTF-- -Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode doesn't allow any extra arguments in %s -bool(false) +--EXPECT-- +PDO::query() expects exactly 2 argument for the fetch mode provided, 4 given PDO::query(): Argument #2 ($fetch_mode) must be of type ?int, string given - -Warning: PDO::query(): SQLSTATE[HY000]: General error: too many arguments in %s -bool(false) - -Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode requires the object parameter in %s -bool(false) - -Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode requires the colno argument in %s -bool(false) - -Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode requires the classname argument in %s -bool(false) +PDO::query() expects at most 4 arguments, 5 given +PDO::query() expects exactly 3 argument for the fetch mode provided, 2 given +PDO::query() expects exactly 3 argument for the fetch mode provided, 2 given +PDO::query() expects exactly 4 argument for the fetch mode provided, 2 given diff --git a/ext/pdo/tests/pdo_038.phpt b/ext/pdo/tests/pdo_038.phpt index 3ff2d090a8580..5851a9fe6fd7d 100644 --- a/ext/pdo/tests/pdo_038.phpt +++ b/ext/pdo/tests/pdo_038.phpt @@ -32,13 +32,16 @@ switch ($conn->getAttribute(PDO::ATTR_DRIVER_NAME)) { $stmt = $conn->prepare($query); -var_dump(fetchColumn($stmt, -1)); +try { + var_dump(fetchColumn($stmt, -1)); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} var_dump(fetchColumn($stmt, 0)); var_dump(fetchColumn($stmt, 1)); ?> --EXPECTF-- -Warning: PDOStatement::fetchColumn(): SQLSTATE[HY000]: General error: Invalid column index in %s -bool(false) +Column index must be greater than or equal to 0 string(1) "1" Warning: PDOStatement::fetchColumn(): SQLSTATE[HY000]: General error: Invalid column index in %s From c5a4c3eac9b19e4fcbfa2f0e3fc16179a851c111 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 02:43:44 +0100 Subject: [PATCH 04/50] Drop ZPP check in PDO Mysqli test... --- ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt b/ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt index c8f188a738908..eac4b9e4d80af 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt @@ -33,10 +33,6 @@ if (!MySQLPDOTest::isPDOMySQLnd()) if (false !== ($tmp = $stmt->nextRowSet())) printf("[002] Expecting false got %s\n", var_export($tmp, true)); - // TODO: should give a warning, but its PDO, let's ignore the missing warning for now - if (false !== ($tmp = $stmt->nextRowSet(1))) - printf("[003] Expecting false got %s\n", var_export($tmp, true)); - function test_proc1($db) { $stmt = $db->query('SELECT @VERSION as _version'); From 4c75cae57e976c9106ce85832642b1608bb6080e Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 03:31:03 +0100 Subject: [PATCH 05/50] Make certain error conditions always error in PDO::fetchAll() --- ext/pdo/pdo_stmt.c | 132 +++++++++++++++++-------------------- ext/pdo/pdo_stmt.stub.php | 2 +- ext/pdo/pdo_stmt_arginfo.h | 5 +- 3 files changed, 66 insertions(+), 73 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 88121217be560..ec92b0c3967c0 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1286,8 +1286,8 @@ PHP_METHOD(PDOStatement, fetchAll) ZEND_PARSE_PARAMETERS_START(0, 3) Z_PARAM_OPTIONAL Z_PARAM_LONG(how) - Z_PARAM_ZVAL(arg2) - Z_PARAM_ZVAL(ctor_args) + Z_PARAM_ZVAL_OR_NULL(arg2) + Z_PARAM_ARRAY_OR_NULL(ctor_args) ZEND_PARSE_PARAMETERS_END(); PHP_STMT_GET_OBJ; @@ -1301,85 +1301,77 @@ PHP_METHOD(PDOStatement, fetchAll) do_fetch_opt_finish(stmt, 0); - switch(how & ~PDO_FETCH_FLAGS) { - case PDO_FETCH_CLASS: - switch(ZEND_NUM_ARGS()) { - case 0: - case 1: - stmt->fetch.cls.ce = zend_standard_class_def; - break; - case 3: - if (Z_TYPE_P(ctor_args) != IS_NULL && Z_TYPE_P(ctor_args) != IS_ARRAY) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "ctor_args must be either NULL or an array"); - error = 1; - break; - } - if (Z_TYPE_P(ctor_args) != IS_ARRAY || !zend_hash_num_elements(Z_ARRVAL_P(ctor_args))) { - ctor_args = NULL; + /* TODO Would be good to reuse part of pdo_stmt_setup_fetch_mode() in some way */ + + switch (how & ~PDO_FETCH_FLAGS) { + case PDO_FETCH_CLASS: + /* Figure out correct class */ + if (arg2) { + if (Z_TYPE_P(arg2) != IS_STRING) { + zend_argument_type_error(2, "must be string, %s given", zend_zval_type_name(arg2)); + RETURN_THROWS(); + } + stmt->fetch.cls.ce = zend_fetch_class(Z_STR_P(arg2), ZEND_FETCH_CLASS_AUTO); + if (!stmt->fetch.cls.ce) { + zend_argument_type_error(2, "must be a valid class"); + RETURN_THROWS(); + } + } else { + stmt->fetch.cls.ce = zend_standard_class_def; } - /* no break */ - case 2: - if (ctor_args) { + + if (ctor_args && zend_hash_num_elements(Z_ARRVAL_P(ctor_args)) > 0) { ZVAL_COPY_VALUE(&stmt->fetch.cls.ctor_args, ctor_args); /* we're not going to free these */ } else { ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); } - if (Z_TYPE_P(arg2) != IS_STRING) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Invalid class name (should be a string)"); - error = 1; - break; - } else { - stmt->fetch.cls.ce = zend_fetch_class(Z_STR_P(arg2), ZEND_FETCH_CLASS_AUTO); - if (!stmt->fetch.cls.ce) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "could not find user-specified class"); - error = 1; - break; - } - } - } - if (!error) { + do_fetch_class_prepare(stmt); - } - break; + break; - case PDO_FETCH_FUNC: - switch (ZEND_NUM_ARGS()) { - case 0: - case 1: - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "no fetch function specified"); + case PDO_FETCH_FUNC: /* Cannot be a default fetch mode */ + if (ZEND_NUM_ARGS() != 2) { + zend_argument_count_error("%s() expects exactly 2 argument for PDO::FETCH_FUNC, %d given", + ZSTR_VAL(get_active_function_or_method_name()), ZEND_NUM_ARGS()); + RETURN_THROWS(); + } + /* TODO Check it is a callable? */ + ZVAL_COPY_VALUE(&stmt->fetch.func.function, arg2); + if (do_fetch_func_prepare(stmt) == 0) { error = 1; - break; - case 3: - case 2: - ZVAL_COPY_VALUE(&stmt->fetch.func.function, arg2); - if (do_fetch_func_prepare(stmt) == 0) { - error = 1; - } - break; - } - break; - - case PDO_FETCH_COLUMN: - switch(ZEND_NUM_ARGS()) { - case 0: - case 1: - stmt->fetch.column = how & PDO_FETCH_GROUP ? -1 : 0; + } break; - case 2: - convert_to_long(arg2); - stmt->fetch.column = Z_LVAL_P(arg2); + + case PDO_FETCH_COLUMN: + if (ZEND_NUM_ARGS() > 2) { + zend_argument_count_error("%s() expects at most 2 argument for the fetch mode provided, %d given", + ZSTR_VAL(get_active_function_or_method_name()), ZEND_NUM_ARGS()); + RETURN_THROWS(); + } + /* Is column index passed? */ + if (arg2) { + // Reuse convert_to_long(arg2); ? + if (Z_TYPE_P(arg2) != IS_LONG) { + zend_argument_type_error(2, "must be int, %s given", zend_zval_type_name(arg2)); + RETURN_THROWS(); + } + if (Z_LVAL_P(arg2) < 0) { + zend_argument_value_error(2, "must be greater than or equal to 0"); + RETURN_THROWS(); + } + stmt->fetch.column = Z_LVAL_P(arg2); + } else { + stmt->fetch.column = how & PDO_FETCH_GROUP ? -1 : 0; + } break; - case 3: - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Third parameter not allowed for PDO::FETCH_COLUMN"); - error = 1; - } - break; - default: - if (ZEND_NUM_ARGS() > 1) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Extraneous additional parameters"); - error = 1; - } + default: + /* No support for PDO_FETCH_INTO which takes 2 args??? */ + if (ZEND_NUM_ARGS() > 1) { + zend_argument_count_error("%s() expects exactly 1 argument for the fetch mode provided, %d given", + ZSTR_VAL(get_active_function_or_method_name()), ZEND_NUM_ARGS()); + RETURN_THROWS(); + } } flags = how & PDO_FETCH_FLAGS; diff --git a/ext/pdo/pdo_stmt.stub.php b/ext/pdo/pdo_stmt.stub.php index 137586079e41d..a170c21b1e744 100644 --- a/ext/pdo/pdo_stmt.stub.php +++ b/ext/pdo/pdo_stmt.stub.php @@ -35,7 +35,7 @@ public function execute(?array $input_parameters = null) {} public function fetch(int $fetch_style = PDO::FETCH_BOTH, int $cursor_orientation = PDO::FETCH_ORI_NEXT, int $cursor_offset = 0) {} /** @return array|false */ - public function fetchAll(int $fetch_style = PDO::FETCH_BOTH, mixed ...$fetch_args) {} + public function fetchAll(int $fetch_style = PDO::FETCH_BOTH, mixed $arg2 = null, ?array $constructorArguments = []) {} /** @return mixed */ public function fetchColumn(int $column_number = 0) {} diff --git a/ext/pdo/pdo_stmt_arginfo.h b/ext/pdo/pdo_stmt_arginfo.h index ba95ede9c354c..f0199f7c14cb2 100644 --- a/ext/pdo/pdo_stmt_arginfo.h +++ b/ext/pdo/pdo_stmt_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: afff84f6e878c3d8837bf53a4ce3617b9c7fb386 */ + * Stub hash: 6d360d7d412e0ebbbc466f8abf104f3969eae738 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_bindColumn, 0, 0, 2) ZEND_ARG_TYPE_MASK(0, column, MAY_BE_STRING|MAY_BE_LONG, NULL) @@ -46,7 +46,8 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_fetchAll, 0, 0, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, fetch_style, IS_LONG, 0, "PDO::FETCH_BOTH") - ZEND_ARG_VARIADIC_TYPE_INFO(0, fetch_args, IS_MIXED, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, arg2, IS_MIXED, 0, "null") + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, constructorArguments, IS_ARRAY, 1, "[]") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_fetchColumn, 0, 0, 0) From 2cf849b8b40f787bf0f78b3e6f481a153296a58e Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 03:44:26 +0100 Subject: [PATCH 06/50] Unconditonal error for values less than a minimum --- ext/pdo/pdo_stmt.c | 14 +++++++------- .../tests/pdo_mysql_stmt_getcolumnmeta.phpt | 8 ++++++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index ec92b0c3967c0..cc3dde44390a5 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1454,8 +1454,8 @@ static void register_bound_param(INTERNAL_FUNCTION_PARAMETERS, int is_param) /* } else if (param.paramno > 0) { --param.paramno; /* make it zero-based internally */ } else { - pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "Columns/Parameters are 1-based"); - RETURN_FALSE; + zend_argument_value_error(1, "must be greater than or equal to 1"); + RETURN_THROWS(); } if (driver_params) { @@ -1498,8 +1498,8 @@ PHP_METHOD(PDOStatement, bindValue) } else if (param.paramno > 0) { --param.paramno; /* make it zero-based internally */ } else { - pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "Columns/Parameters are 1-based"); - RETURN_FALSE; + zend_argument_value_error(1, "must be greater than or equal to 1"); + RETURN_THROWS(); } ZVAL_COPY(¶m.parameter, parameter); @@ -1685,9 +1685,9 @@ PHP_METHOD(PDOStatement, getColumnMeta) ZEND_PARSE_PARAMETERS_END(); PHP_STMT_GET_OBJ; - if(colno < 0) { - pdo_raise_impl_error(stmt->dbh, stmt, "42P10", "column number must be non-negative"); - RETURN_FALSE; + if (colno < 0) { + zend_argument_value_error(1, "must be greater than or equal to 0"); + RETURN_THROWS(); } if (!stmt->methods->get_column_meta) { diff --git a/ext/pdo_mysql/tests/pdo_mysql_stmt_getcolumnmeta.phpt b/ext/pdo_mysql/tests/pdo_mysql_stmt_getcolumnmeta.phpt index 44f0a0ebb18d2..b5b0275f04f7d 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_stmt_getcolumnmeta.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_stmt_getcolumnmeta.phpt @@ -32,8 +32,11 @@ try { $stmt->execute(); // invalid offset - if (false !== ($tmp = @$stmt->getColumnMeta(-1))) - printf("[004] Expecting false got %s\n", var_export($tmp, true)); + try { + $stmt->getColumnMeta(-1); + } catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; + } $emulated = $stmt->getColumnMeta(0); @@ -299,5 +302,6 @@ $db->exec('DROP TABLE IF EXISTS test'); print "done!"; ?> --EXPECT-- +PDOStatement::getColumnMeta(): Argument #1 ($column) must be greater than or equal to 0 Testing native PS... done! From 220d89f8e67d00bb9e633a9058f6e77d37c8ea87 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 03:52:36 +0100 Subject: [PATCH 07/50] Add todo comments --- ext/pdo/pdo_stmt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index cc3dde44390a5..b30633da6914c 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -265,6 +265,7 @@ static int really_register_bound_param(struct pdo_bound_param_data *param, pdo_s /* if you prepare and then execute passing an array of params keyed by names, * then this will trigger, and we don't want that */ if (param->paramno == -1) { + /* Should this always be an Error? */ char *tmp; spprintf(&tmp, 0, "Did not find column name '%s' in the defined columns; it will not be bound", ZSTR_VAL(param->name)); pdo_raise_impl_error(stmt->dbh, stmt, "HY000", tmp); @@ -464,6 +465,7 @@ static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, int *typ } if (colno >= stmt->column_count) { + /* TODO Should this be a ValueError? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Invalid column index"); ZVAL_FALSE(dest); From b2bd8b69ca3c724fd365f78505f16606be8ab0c9 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 04:11:16 +0100 Subject: [PATCH 08/50] ValueError for negative column index --- ext/pdo/pdo_stmt.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index b30633da6914c..41d09fa98042f 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -815,23 +815,27 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h break; case PDO_FETCH_COLUMN: - if (colno >= 0 && colno < stmt->column_count) { - if (flags == PDO_FETCH_GROUP && stmt->fetch.column == -1) { - fetch_value(stmt, return_value, 1, NULL); - } else if (flags == PDO_FETCH_GROUP && colno) { - fetch_value(stmt, return_value, 0, NULL); - } else { - fetch_value(stmt, return_value, colno, NULL); - } - if (!return_all) { - return 1; - } else { - break; - } - } else { + if (colno < 0 ) { + zend_value_error("Column index must be greater than or equal to 0"); + return false; + } + /* TODO Always a ValueError? */ + if (colno >= stmt->column_count) { pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Invalid column index"); + return false; } - return 0; + + if (flags == PDO_FETCH_GROUP && stmt->fetch.column == -1) { + fetch_value(stmt, return_value, 1, NULL); + } else if (flags == PDO_FETCH_GROUP && colno) { + fetch_value(stmt, return_value, 0, NULL); + } else { + fetch_value(stmt, return_value, colno, NULL); + } + if (!return_all) { + return 1; + } + break; case PDO_FETCH_OBJ: object_init_ex(return_value, ZEND_STANDARD_CLASS_DEF_PTR); From 156ec136efaa3fb0e71b353b8f3faa8b68a715b0 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 04:26:47 +0100 Subject: [PATCH 09/50] Assertion + always throw on invalid Fetch mode --- ext/pdo/pdo_stmt.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 41d09fa98042f..0ec5cf9f9ece4 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -923,10 +923,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h } } break; - - default: - /* shouldn't happen */ - return 0; + EMPTY_SWITCH_DEFAULT_CASE(); } if (return_all && how != PDO_FETCH_KEY_PAIR) { @@ -1052,9 +1049,8 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h default: zval_ptr_dtor(&val); - pdo_raise_impl_error(stmt->dbh, stmt, "22003", "mode is out of range"); + zend_value_error("Fetch mode must be a bitmask of PDO::FETCH_* constants"); return 0; - break; } } From 7bcf852f4277befa81274bd7065e7504f4225cca Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 04:31:47 +0100 Subject: [PATCH 10/50] ValueError for incorrect Fetch Flag usage --- ext/pdo/pdo_stmt.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 0ec5cf9f9ece4..7be7b9d694a62 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1138,24 +1138,24 @@ static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode switch(mode) { case PDO_FETCH_FUNC: if (!fetch_all) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "PDO::FETCH_FUNC is only allowed in PDOStatement::fetchAll()"); + zend_argument_value_error(mode_arg_num, "can only use PDO::FETCH_FUNC in PDOStatement::fetchAll()"); return 0; } return 1; case PDO_FETCH_LAZY: if (fetch_all) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "PDO::FETCH_LAZY can't be used with PDOStatement::fetchAll()"); + zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_LAZY in PDOStatement::fetchAll()"); return 0; } /* fall through */ default: if ((flags & PDO_FETCH_SERIALIZE) == PDO_FETCH_SERIALIZE) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "PDO::FETCH_SERIALIZE can only be used together with PDO::FETCH_CLASS"); + zend_argument_value_error(mode_arg_num, "must use PDO::FETCH_SERIALIZE with PDO::FETCH_CLASS"); return 0; } if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "PDO::FETCH_CLASSTYPE can only be used together with PDO::FETCH_CLASS"); + zend_argument_value_error(mode_arg_num, "must use PDO::FETCH_CLASSTYPE with PDO::FETCH_CLASS"); return 0; } if (mode >= PDO_FETCH__MAX) { From d92274a6b40bb62b95aaded508c91bf59430aade Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 04:41:01 +0100 Subject: [PATCH 11/50] Make invalid user callable throw a type error --- ext/pdo/pdo_stmt.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 7be7b9d694a62..dd3421fbc3d4b 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -689,12 +689,12 @@ static int make_callable_ex(pdo_stmt_t *stmt, zval *callable, zend_fcall_info * if (zend_fcall_info_init(callable, 0, fci, fcc, NULL, &is_callable_error) == FAILURE) { if (is_callable_error) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", is_callable_error); + zend_type_error("%s", is_callable_error); efree(is_callable_error); } else { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "user-supplied function must be a valid callback"); + zend_type_error("user-supplied function must be a valid callback"); } - return 0; + return false; } if (is_callable_error) { /* Possible error message */ @@ -704,20 +704,20 @@ static int make_callable_ex(pdo_stmt_t *stmt, zval *callable, zend_fcall_info * fci->param_count = num_args; /* probably less */ fci->params = safe_emalloc(sizeof(zval), num_args, 0); - return 1; + return true; } /* }}} */ -static int do_fetch_func_prepare(pdo_stmt_t *stmt) /* {{{ */ +static bool do_fetch_func_prepare(pdo_stmt_t *stmt) /* {{{ */ { zend_fcall_info *fci = &stmt->fetch.cls.fci; zend_fcall_info_cache *fcc = &stmt->fetch.cls.fcc; if (!make_callable_ex(stmt, &stmt->fetch.func.function, fci, fcc, stmt->column_count)) { - return 0; + return false; } else { stmt->fetch.func.values = safe_emalloc(sizeof(zval), stmt->column_count, 0); - return 1; + return true; } } /* }}} */ @@ -1339,8 +1339,8 @@ PHP_METHOD(PDOStatement, fetchAll) } /* TODO Check it is a callable? */ ZVAL_COPY_VALUE(&stmt->fetch.func.function, arg2); - if (do_fetch_func_prepare(stmt) == 0) { - error = 1; + if (do_fetch_func_prepare(stmt) == false) { + RETURN_THROWS(); } break; From 4028d9f6b75f91dd35ecfee11a28992b69025351 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 04:53:40 +0100 Subject: [PATCH 12/50] More TODOs --- ext/pdo/pdo_stmt.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index dd3421fbc3d4b..718f886909d8b 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -62,6 +62,7 @@ static inline int rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_pa param->name = zend_string_init(name, strlen(name), 0); return 1; } + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "parameter was not defined"); return 0; } @@ -72,12 +73,14 @@ static inline int rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_pa continue; } if (param->paramno >= 0) { + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "IM001", "PDO refuses to handle repeating the same :named parameter for multiple positions with this driver, as it might be unsafe to do so. Consider using a separate name for each parameter instead"); return -1; } param->paramno = position; return 1; } ZEND_HASH_FOREACH_END(); + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "parameter was not defined"); return 0; } @@ -267,6 +270,7 @@ static int really_register_bound_param(struct pdo_bound_param_data *param, pdo_s if (param->paramno == -1) { /* Should this always be an Error? */ char *tmp; + /* TODO Error? */ spprintf(&tmp, 0, "Did not find column name '%s' in the defined columns; it will not be bound", ZSTR_VAL(param->name)); pdo_raise_impl_error(stmt->dbh, stmt, "HY000", tmp); efree(tmp); @@ -674,7 +678,7 @@ static int do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */ fcc->called_scope = ce; return 1; } else if (!Z_ISUNDEF(stmt->fetch.cls.ctor_args)) { - /* Promote to Error? */ + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "user-supplied class does not have a constructor, use NULL for the ctor_params parameter, or simply omit it"); return 0; } else { @@ -868,6 +872,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h } ce = stmt->fetch.cls.ce; if (!ce) { + /* TODO ArgumentCountError? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch class specified"); return 0; } @@ -885,6 +890,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h stmt->fetch.cls.fci.object = Z_OBJ_P(return_value); stmt->fetch.cls.fcc.object = Z_OBJ_P(return_value); if (zend_call_function(&stmt->fetch.cls.fci, &stmt->fetch.cls.fcc) == FAILURE) { + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "could not call class constructor"); return 0; } else { @@ -899,6 +905,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h case PDO_FETCH_INTO: if (Z_ISUNDEF(stmt->fetch.into)) { + /* TODO ArgumentCountError? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch-into object specified."); return 0; break; @@ -913,6 +920,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h case PDO_FETCH_FUNC: if (Z_ISUNDEF(stmt->fetch.func.function)) { + /* TODO ArgumentCountError? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch function specified"); return 0; } @@ -1060,6 +1068,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h stmt->fetch.cls.fci.object = Z_OBJ_P(return_value); stmt->fetch.cls.fcc.object = Z_OBJ_P(return_value); if (zend_call_function(&stmt->fetch.cls.fci, &stmt->fetch.cls.fcc) == FAILURE) { + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "could not call class constructor"); return 0; } else { @@ -1080,6 +1089,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h stmt->fetch.func.fci.param_count = idx; stmt->fetch.func.fci.retval = &retval; if (zend_call_function(&stmt->fetch.func.fci, &stmt->fetch.func.fcc) == FAILURE) { + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "could not call user-supplied function"); return 0; } else { @@ -2032,6 +2042,7 @@ static zval *dbstmt_prop_write(zend_object *object, zend_string *name, zval *val pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); if (strcmp(ZSTR_VAL(name), "queryString") == 0) { + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "property queryString is read only"); return value; } else { @@ -2044,6 +2055,7 @@ static void dbstmt_prop_delete(zend_object *object, zend_string *name, void **ca pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); if (strcmp(ZSTR_VAL(name), "queryString") == 0) { + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "property queryString is read only"); } else { zend_std_unset_property(object, name, cache_slot); From 1615a93d8c68e0f5e2c617cfc2e64fcae85fc0f7 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 04:59:59 +0100 Subject: [PATCH 13/50] Todo everything... --- ext/pdo/pdo_dbh.c | 17 +++++++++++++++++ ext/pdo/pdo_sql_parser.re | 4 ++++ 2 files changed, 21 insertions(+) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index a2d01f33ef3ff..c07349966bcdd 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -426,10 +426,12 @@ static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry { if (!Z_ISUNDEF_P(ctor_args)) { if (Z_TYPE_P(ctor_args) != IS_ARRAY) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "constructor arguments must be passed as an array"); return NULL; } if (!dbstmt_ce->constructor) { + /* TODO Error? */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement does not accept constructor arguments"); return NULL; } @@ -506,6 +508,7 @@ PHP_METHOD(PDO, prepare) || Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL ) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); " "the classname must be a string specifying an existing class" @@ -515,12 +518,14 @@ PHP_METHOD(PDO, prepare) } dbstmt_ce = pce; if (!instanceof_function(dbstmt_ce, pdo_dbstmt_ce)) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement class must be derived from PDOStatement"); PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } if (dbstmt_ce->constructor && !(dbstmt_ce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) { + /* TODO Always TError */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement class cannot have a public constructor"); PDO_HANDLE_DBH_ERR(); @@ -528,6 +533,7 @@ PHP_METHOD(PDO, prepare) } if ((item = zend_hash_index_find(Z_ARRVAL_P(opt), 1)) != NULL) { if (Z_TYPE_P(item) != IS_ARRAY) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "PDO::ATTR_STATEMENT_CLASS requires format array(classname, ctor_args); " "ctor_args must be an array" @@ -546,6 +552,7 @@ PHP_METHOD(PDO, prepare) if (!pdo_stmt_instantiate(dbh, return_value, dbstmt_ce, &ctor_args)) { if (EXPECTED(!EG(exception))) { + /* TODO Error? */ pdo_raise_impl_error(dbh, NULL, "HY000", "failed to instantiate user-supplied statement class" ); @@ -679,6 +686,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* { zend_long lval; +/* TODO Always TypeError (also restrict to actual integer?) */ #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) { \ pdo_raise_impl_error(dbh, NULL, "HY000", "attribute value must be an integer"); \ @@ -697,6 +705,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* dbh->error_mode = lval; return SUCCESS; default: + /* TODO Always ValueError */ pdo_raise_impl_error(dbh, NULL, "HY000", "invalid error mode"); PDO_HANDLE_DBH_ERR(); return FAILURE; @@ -713,6 +722,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* dbh->desired_case = lval; return SUCCESS; default: + /* TODO Always ValueError */ pdo_raise_impl_error(dbh, NULL, "HY000", "invalid case folding mode"); PDO_HANDLE_DBH_ERR(); return FAILURE; @@ -729,6 +739,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* 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) { + /* TODO Always ValueError */ pdo_raise_impl_error(dbh, NULL, "HY000", "FETCH_INTO and FETCH_CLASS are not yet supported as default fetch modes"); return FAILURE; } @@ -738,6 +749,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* } lval = zval_get_long(value); if (lval == PDO_FETCH_USE_DEFAULT) { + /* TODO Always ValueError */ pdo_raise_impl_error(dbh, NULL, "HY000", "invalid fetch mode type"); return FAILURE; } @@ -766,6 +778,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* || Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL ) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); " "the classname must be a string specifying an existing class" @@ -774,12 +787,14 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* return FAILURE; } if (!instanceof_function(pce, pdo_dbstmt_ce)) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement class must be derived from PDOStatement"); PDO_HANDLE_DBH_ERR(); return FAILURE; } if (pce->constructor && !(pce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement class cannot have a public constructor"); PDO_HANDLE_DBH_ERR(); @@ -792,6 +807,7 @@ static int 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) { + /* TODO Always TypeError */ pdo_raise_impl_error(dbh, NULL, "HY000", "PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); " "ctor_args must be an array" @@ -928,6 +944,7 @@ PHP_METHOD(PDO, exec) ZEND_PARSE_PARAMETERS_END(); if (!statement_len) { + /* TODO ValueError */ pdo_raise_impl_error(dbh, NULL, "HY000", "trying to execute an empty query"); RETURN_FALSE; } diff --git a/ext/pdo/pdo_sql_parser.re b/ext/pdo/pdo_sql_parser.re index ee0ce7168d384..1a132cb7b22fb 100644 --- a/ext/pdo/pdo_sql_parser.re +++ b/ext/pdo/pdo_sql_parser.re @@ -151,6 +151,7 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, const char *inquery, size_t inque /* did the query make sense to me? */ if (query_type == (PDO_PLACEHOLDER_NAMED|PDO_PLACEHOLDER_POSITIONAL)) { /* they mixed both types; punt */ + /* TODO Always Error */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "mixed named and positional parameters"); ret = -1; goto clean_up; @@ -192,6 +193,7 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, const char *inquery, size_t inque goto safe; } } + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "number of bound variables does not match number of tokens"); ret = -1; goto clean_up; @@ -222,6 +224,7 @@ safe: if (param == NULL) { /* parameter was not defined */ ret = -1; + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "parameter was not defined"); goto clean_up; } @@ -257,6 +260,7 @@ safe: zend_string_release_ex(buf, 0); } } else { + /* TODO Always TypeError */ pdo_raise_impl_error(stmt->dbh, stmt, "HY105", "Expected a stream resource"); ret = -1; goto clean_up; From 2f9bb0eef3ca8ca19a811dd077f4d07e07e1d075 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 05:07:08 +0100 Subject: [PATCH 14/50] ValueError for empty query in PDO::exec() --- ext/pdo/pdo_dbh.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index c07349966bcdd..197ff51c4f85b 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -943,11 +943,11 @@ PHP_METHOD(PDO, exec) Z_PARAM_STRING(statement, statement_len) ZEND_PARSE_PARAMETERS_END(); - if (!statement_len) { - /* TODO ValueError */ - pdo_raise_impl_error(dbh, NULL, "HY000", "trying to execute an empty query"); - RETURN_FALSE; + if (statement_len == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); } + PDO_DBH_CLEAR_ERR(); PDO_CONSTRUCT_CHECK; ret = dbh->methods->doer(dbh, statement, statement_len); From ad8a780b149a4ba4bde40ea530521729a3c98504 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 05:31:13 +0100 Subject: [PATCH 15/50] Preliminary Type/ValueError for setting PDO attributes --- ext/pdo/pdo_dbh.c | 17 ++----- .../tests/pdo_mysql_attr_errmode.phpt | 48 +++++++++---------- 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 197ff51c4f85b..31f14c2cb68cf 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -689,8 +689,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* /* TODO Always TypeError (also restrict to actual integer?) */ #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) { \ - pdo_raise_impl_error(dbh, NULL, "HY000", "attribute value must be an integer"); \ - PDO_HANDLE_DBH_ERR(); \ + zend_type_error("Attribute value must be int for selected attribute, %s given", zend_zval_type_name(value)); \ return FAILURE; \ } \ @@ -705,9 +704,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* dbh->error_mode = lval; return SUCCESS; default: - /* TODO Always ValueError */ - pdo_raise_impl_error(dbh, NULL, "HY000", "invalid error mode"); - PDO_HANDLE_DBH_ERR(); + zend_value_error("Error mode must be one of PDO::ERRMODE_* constants"); return FAILURE; } return FAILURE; @@ -722,9 +719,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* dbh->desired_case = lval; return SUCCESS; default: - /* TODO Always ValueError */ - pdo_raise_impl_error(dbh, NULL, "HY000", "invalid case folding mode"); - PDO_HANDLE_DBH_ERR(); + zend_value_error("Case folding mode must be one of PDO::CASE_* constants"); return FAILURE; } return FAILURE; @@ -739,8 +734,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* 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) { - /* TODO Always ValueError */ - pdo_raise_impl_error(dbh, NULL, "HY000", "FETCH_INTO and FETCH_CLASS are not yet supported as default fetch modes"); + zend_value_error("PDO::FETCH_INTO and PDO::FETCH_CLASS cannot be set as default fetch mode"); return FAILURE; } } @@ -749,8 +743,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* } lval = zval_get_long(value); if (lval == PDO_FETCH_USE_DEFAULT) { - /* TODO Always ValueError */ - pdo_raise_impl_error(dbh, NULL, "HY000", "invalid fetch mode type"); + zend_value_error("Fetch mode must be a bitmask of PDO::FETCH_* constants"); return FAILURE; } dbh->default_fetch_type = lval; diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt index 0d6cee356ebbd..72a6004e50852 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt @@ -14,29 +14,27 @@ error_reporting=E_ALL require_once(__DIR__ . DIRECTORY_SEPARATOR . 'mysql_pdo_test.inc'); $db = MySQLPDOTest::factory(); - $valid = array(PDO::ERRMODE_SILENT, PDO::ERRMODE_WARNING, PDO::ERRMODE_EXCEPTION); - do { - $invalid = mt_rand(-1000, 1000); - } while (in_array($invalid, $valid)); - - - $tmp = array(); - if (false != @$db->setAttribute(PDO::ATTR_ERRMODE, $tmp)) - printf("[001] Maybe PDO could indicate that this is not a proper way of setting the ERRMODE...\n"); - - $tmp = new stdClass(); - $ret = @$db->setAttribute(PDO::ATTR_ERRMODE, $tmp); - if (false != $ret) - printf("[002] Maybe PDO could indicate that this is not a proper way of setting the ERRMODE...%s\n", - var_export($ret, true)); - - $ret = @$db->setAttribute(PDO::ATTR_ERRMODE, 'pdo'); - if (false != $ret) - printf("[003] Maybe PDO could indicate that this is not a proper way of setting the ERRMODE...%s\n", - var_export($ret, true)); - - if (false != @$db->setAttribute(PDO::ATTR_ERRMODE, $invalid)) - printf("[004] Invalid ERRMODE should be rejected\n"); + try { + $db->setAttribute(PDO::ATTR_ERRMODE, []); + } catch (\Error $e) { + echo get_class($e), ': ', $e->getMessage(), \PHP_EOL; + } + try { + $db->setAttribute(PDO::ATTR_ERRMODE, new stdClass()); + } catch (\Error $e) { + 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; + } + try { + $db->setAttribute(PDO::ATTR_ERRMODE, 1000); + } catch (\Error $e) { + echo get_class($e), ': ', $e->getMessage(), \PHP_EOL; + } $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT); // no message for any PDO call but... @@ -160,7 +158,9 @@ error_reporting=E_ALL print "done!\n"; ?> --EXPECTF-- -[003] Maybe PDO could indicate that this is not a proper way of setting the ERRMODE...true +TypeError: Attribute value must be int for selected attribute, array given +TypeError: Attribute value must be int for selected attribute, stdClass given +ValueError: Error mode must be one of 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 From ba6ce47fb9a0871cfcaa1c6eca015bc02c71a61e Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 05:38:34 +0100 Subject: [PATCH 16/50] Attempt to fix SQLite test --- ext/pdo/tests/bug_44159.phpt | 24 ++-- ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt | 111 ++++++++++--------- 2 files changed, 73 insertions(+), 62 deletions(-) diff --git a/ext/pdo/tests/bug_44159.phpt b/ext/pdo/tests/bug_44159.phpt index 0e1116d58863e..5687f00d313d5 100644 --- a/ext/pdo/tests/bug_44159.phpt +++ b/ext/pdo/tests/bug_44159.phpt @@ -17,9 +17,21 @@ $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING); $attrs = array(PDO::ATTR_STATEMENT_CLASS, PDO::ATTR_STRINGIFY_FETCHES, PDO::NULL_TO_STRING); foreach ($attrs as $attr) { - var_dump($pdo->setAttribute($attr, NULL)); - var_dump($pdo->setAttribute($attr, 1)); - var_dump($pdo->setAttribute($attr, 'nonsense')); + try { + var_dump($pdo->setAttribute($attr, NULL)); + } catch (\Error $e) { + echo get_class($e), ': ', $e->getMessage(), \PHP_EOL; + } + try { + var_dump($pdo->setAttribute($attr, 1)); + } catch (\Error $e) { + echo get_class($e), ': ', $e->getMessage(), \PHP_EOL; + } + try { + var_dump($pdo->setAttribute($attr, 'nonsense')); + } catch (\Error $e) { + echo get_class($e), ': ', $e->getMessage(), \PHP_EOL; + } } @unlink(__DIR__."/foo.db"); @@ -40,11 +52,7 @@ Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: PDO::ATTR_STATEMEN Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d bool(false) - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: attribute value must be an integer in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) +TypeError: Attribute value must be int for selected attribute, null given bool(true) bool(true) bool(true) diff --git a/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt b/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt index 4600b7935be09..3910d8cc4dd69 100644 --- a/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt +++ b/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt @@ -20,20 +20,40 @@ $st->fetchAll(PDO::FETCH_FUNC, function($x, $y) use ($st) { var_dump($st); print $st = $db->query('SELECT name FROM testing'); var_dump($st->fetchAll(PDO::FETCH_FUNC, 'strtoupper')); -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, 'nothing')); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, 'nothing')); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, '')); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, '')); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, NULL)); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, NULL)); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, 1)); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, 1)); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, array('self', 'foo'))); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, array('self', 'foo'))); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} class foo { public function method($x) { @@ -64,14 +84,26 @@ new bar($db); $st = $db->query('SELECT * FROM testing'); var_dump($st->fetchAll(PDO::FETCH_FUNC, array('bar', 'test'))); -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, array('bar', 'test2'))); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, array('bar', 'test2'))); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, array('bar', 'test3'))); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, array('bar', 'test3'))); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} -$st = $db->query('SELECT * FROM testing'); -var_dump($st->fetchAll(PDO::FETCH_FUNC, array('bar', 'inexistent'))); +try { + $st = $db->query('SELECT * FROM testing'); + var_dump($st->fetchAll(PDO::FETCH_FUNC, array('bar', 'inexistent'))); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} ?> --EXPECTF-- @@ -84,38 +116,21 @@ object(PDOStatement)#%d (1) { ["queryString"]=> string(21) "SELECT * FROM testing" } -data: 2, +data: 2, array(2) { [0]=> string(3) "PHP" [1]=> string(0) "" } - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: function "nothing" not found or invalid function name in %s on line %d - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: function "" not found or invalid function name in %s on line %d - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: no array or string given in %s on line %d - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: no array or string given in %s on line %d - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: cannot access "self" when no class scope is active in %s on line %d +function "nothing" not found or invalid function name +function "" not found or invalid function name +no array or string given +no array or string given Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d bool(false) +cannot access "self" when no class scope is active array(2) { [0]=> string(9) "--- 1 ---" @@ -128,18 +143,6 @@ array(2) { [1]=> string(4) "2---" } - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: non-static method bar::test2() cannot be called statically in %s on line %d - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: non-static method bar::test3() cannot be called statically in %s on line %d - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error: class bar does not have a method "inexistent" in %s on line %d - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) +non-static method bar::test2() cannot be called statically +non-static method bar::test3() cannot be called statically +class bar does not have a method "inexistent" in %s on line %d From dd7c891b8aae21e33e3cf39684ce1b1490f6cb29 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 15:54:10 +0100 Subject: [PATCH 17/50] TypeErrors for invalid attribute options --- ext/pdo/pdo_dbh.c | 38 ++++++-------- ext/pdo/tests/bug_44159.phpt | 40 ++++++-------- .../tests/pdo_mysql_attr_oracle_nulls.phpt | 30 ++++++----- .../tests/pdo_mysql_attr_statement_class.phpt | 52 ++++++++++++------- 4 files changed, 81 insertions(+), 79 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 31f14c2cb68cf..b6990e08fe72e 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -766,28 +766,26 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* PDO_HANDLE_DBH_ERR(); return FAILURE; } - if (Z_TYPE_P(value) != IS_ARRAY - || (item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL - || Z_TYPE_P(item) != IS_STRING - || (pce = zend_lookup_class(Z_STR_P(item))) == NULL - ) { - /* TODO Always TypeError */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); " - "the classname must be a string specifying an existing class" - ); - PDO_HANDLE_DBH_ERR(); + if (Z_TYPE_P(value) != IS_ARRAY) { + zend_type_error("PDO::ATTR_STATEMENT_CLASS's value must be of type array, %s given", + zend_zval_type_name(value)); + return FAILURE; + } + if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL) { + zend_value_error("PDO::ATTR_STATEMENT_CLASS's value must be an array with the following " + "format array(classname, array(ctor_args))"); + return FAILURE; + } + if (Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL) { + zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be a valid class"); return FAILURE; } if (!instanceof_function(pce, pdo_dbstmt_ce)) { - /* TODO Always TypeError */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "user-supplied statement class must be derived from PDOStatement"); - PDO_HANDLE_DBH_ERR(); + zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement"); return FAILURE; } if (pce->constructor && !(pce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) { - /* TODO Always TypeError */ + /* TODO Always Error? */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement class cannot have a public constructor"); PDO_HANDLE_DBH_ERR(); @@ -800,12 +798,8 @@ static int 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) { - /* TODO Always TypeError */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); " - "ctor_args must be an array" - ); - PDO_HANDLE_DBH_ERR(); + zend_type_error("PDO::ATTR_STATEMENT_CLASS's ctor_args must be ?array, %s given", + zend_zval_type_name(value)); return FAILURE; } ZVAL_COPY(&dbh->def_stmt_ctor_args, item); diff --git a/ext/pdo/tests/bug_44159.phpt b/ext/pdo/tests/bug_44159.phpt index 5687f00d313d5..5d0a509b11613 100644 --- a/ext/pdo/tests/bug_44159.phpt +++ b/ext/pdo/tests/bug_44159.phpt @@ -2,16 +2,17 @@ PDO Common: Bug #44159 (Crash: $pdo->setAttribute(PDO::STATEMENT_ATTR_CLASS, NULL)) --SKIPIF-- --FILE-- setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING); $attrs = array(PDO::ATTR_STATEMENT_CLASS, PDO::ATTR_STRINGIFY_FETCHES, PDO::NULL_TO_STRING); @@ -37,24 +38,13 @@ foreach ($attrs as $attr) { @unlink(__DIR__."/foo.db"); ?> ---EXPECTF-- -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); the classname must be a string specifying an existing class in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); the classname must be a string specifying an existing class in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); the classname must be a string specifying an existing class in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) +--EXPECT-- +TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, null given +TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, int given +TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given TypeError: Attribute value must be int for selected attribute, null given bool(true) bool(true) -bool(true) -bool(true) -bool(true) +bool(false) +bool(false) +bool(false) diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt index 694a0394415ca..192c6eb1f979e 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt @@ -12,16 +12,22 @@ MySQLPDOTest::skip(); $db = MySQLPDOTest::factory(); MySQLPDOTest::createTestTable($db); - $tmp = array(); - if (false !== @$db->setAttribute(PDO::ATTR_ORACLE_NULLS, $tmp)) - printf("[001] Maybe PDO could indicate that this is not a proper way of setting ATTR_ORACLE_NULLS...\n"); - - $tmp = new stdClass(); - if (false !== @$db->setAttribute(PDO::ATTR_ORACLE_NULLS, $tmp)); - printf("[002] Maybe PDO could indicate that this is not a proper way of setting ATTR_ORACLE_NULLS...\n"); - - if (false !== @$db->setAttribute(PDO::ATTR_ORACLE_NULLS, 'pdo')) - printf("[003] Maybe PDO could indicate that this is not a proper way of setting ATTR_ORACLE_NULLS...\n"); + try { + $db->setAttribute(PDO::ATTR_ORACLE_NULLS, []); + } catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; + } + try { + $db->setAttribute(PDO::ATTR_ORACLE_NULLS, new stdClass()); + } catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; + } + try { + /* Currently passes... */ + $db->setAttribute(PDO::ATTR_ORACLE_NULLS, 'pdo'); + } catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; + } $db->setAttribute(PDO::ATTR_ORACLE_NULLS, 1); $stmt = $db->query("SELECT NULL AS z, '' AS a, ' ' AS b, TRIM(' ') as c, ' d' AS d, '" . chr(0) . " e' AS e"); @@ -82,8 +88,8 @@ MySQLPDOTest::skip(); print "done!"; ?> --EXPECTF-- -[002] Maybe PDO could indicate that this is not a proper way of setting ATTR_ORACLE_NULLS... -[003] Maybe PDO could indicate that this is not a proper way of setting ATTR_ORACLE_NULLS... +Attribute value must be int for selected attribute, array given +Attribute value must be int for selected attribute, stdClass given array(1) { [0]=> array(6) { diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt index 66df0cda459a1..d4d64c5eb3c11 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt @@ -16,15 +16,22 @@ $db = MySQLPDOTest::factory(); $default = $db->getAttribute(PDO::ATTR_STATEMENT_CLASS); var_dump($default); - if (false !== ($tmp = @$db->setAttribute(PDO::ATTR_STATEMENT_CLASS, 'foo'))) - printf("[002] Expecting boolean/false got %s\n", var_export($tmp, true)); - - if (false !== ($tmp = @$db->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('classname')))) - printf("[003] Expecting boolean/false got %s\n", var_export($tmp, true)); - + try { + $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, 'foo'); + } catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; + } + try { + $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['classname']); + } catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; + } // unknown class - if (false !== ($tmp = $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('classname', array())))) - printf("[004] Expecting boolean/false got %s\n", var_export($tmp, true)); + try { + $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['classname', []]); + } catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; + } // class not derived from PDOStatement class myclass { @@ -32,8 +39,12 @@ $db = MySQLPDOTest::factory(); printf("myclass\n"); } } - if (false !== ($tmp = $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('myclass', array())))) - printf("[005] Expecting boolean/false got %s\n", var_export($tmp, true)); + + try { + $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['myclass', []]); + } catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; + } // public constructor not allowed class mystatement extends PDOStatement { @@ -42,8 +53,13 @@ $db = MySQLPDOTest::factory(); } } - if (false !== ($tmp = $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('mystatement', array())))) - printf("[006] Expecting boolean/false got %s\n", var_export($tmp, true)); + try { + if (false !== ($tmp = $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['mystatement', []]))) + printf("[006] Expecting boolean/false got %s\n", var_export($tmp, true)); + } catch (\Error $e) { + echo get_class($e), ': ', $e->getMessage(), \PHP_EOL; + } + // ... but a public destructor is allowed class mystatement2 extends PDOStatement { @@ -109,14 +125,10 @@ array(1) { [0]=> string(12) "PDOStatement" } - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); the classname must be a string specifying an existing class in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: user-supplied statement class must be derived from PDOStatement in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d +PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given +PDO::ATTR_STATEMENT_CLASS's class must be a valid class +PDO::ATTR_STATEMENT_CLASS's class must be a valid class +PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: user-supplied statement class cannot have a public constructor in %s on line %d From d1d6393b010e7ebc4e4f62c9fa17b06bbef8f0ce Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 16:14:13 +0100 Subject: [PATCH 18/50] Type Error for invalid options in PDO::prepare() --- ext/pdo/pdo_dbh.c | 56 ++++++++++++++++++++--------------------------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index b6990e08fe72e..bdfa098546554 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -425,11 +425,8 @@ PHP_METHOD(PDO, __construct) static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry *dbstmt_ce, zval *ctor_args) /* {{{ */ { if (!Z_ISUNDEF_P(ctor_args)) { - if (Z_TYPE_P(ctor_args) != IS_ARRAY) { - /* TODO Always TypeError */ - pdo_raise_impl_error(dbh, NULL, "HY000", "constructor arguments must be passed as an array"); - return NULL; - } + /* This implies an error within PDO if this does not hold */ + ZEND_ASSERT(Z_TYPE_P(ctor_args) == IS_ARRAY); if (!dbstmt_ce->constructor) { /* TODO Error? */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement does not accept constructor arguments"); @@ -489,7 +486,7 @@ PHP_METHOD(PDO, prepare) pdo_stmt_t *stmt; char *statement; size_t statement_len; - zval *options = NULL, *opt, *item, ctor_args; + zval *options = NULL, *value, *item, ctor_args; zend_class_entry *dbstmt_ce, *pce; pdo_dbh_object_t *dbh_obj = Z_PDO_OBJECT_P(ZEND_THIS); pdo_dbh_t *dbh = dbh_obj->inner; @@ -503,43 +500,38 @@ PHP_METHOD(PDO, prepare) PDO_DBH_CLEAR_ERR(); PDO_CONSTRUCT_CHECK; - if (ZEND_NUM_ARGS() > 1 && (opt = zend_hash_index_find(Z_ARRVAL_P(options), PDO_ATTR_STATEMENT_CLASS)) != NULL) { - if (Z_TYPE_P(opt) != IS_ARRAY || (item = zend_hash_index_find(Z_ARRVAL_P(opt), 0)) == NULL - || Z_TYPE_P(item) != IS_STRING - || (pce = zend_lookup_class(Z_STR_P(item))) == NULL - ) { - /* TODO Always TypeError */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "PDO::ATTR_STATEMENT_CLASS requires format array(classname, array(ctor_args)); " - "the classname must be a string specifying an existing class" - ); - PDO_HANDLE_DBH_ERR(); - RETURN_FALSE; + if (options && (value = zend_hash_index_find(Z_ARRVAL_P(options), PDO_ATTR_STATEMENT_CLASS)) != NULL) { + if (Z_TYPE_P(value) != IS_ARRAY) { + zend_type_error("PDO::ATTR_STATEMENT_CLASS's value must be of type array, %s given", + zend_zval_type_name(value)); + RETURN_THROWS(); + } + if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL) { + zend_value_error("PDO::ATTR_STATEMENT_CLASS's value must be an array with the following " + "format array(classname, array(ctor_args))"); + RETURN_THROWS(); + } + if (Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL) { + zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be a valid class"); + RETURN_THROWS(); } dbstmt_ce = pce; if (!instanceof_function(dbstmt_ce, pdo_dbstmt_ce)) { - /* TODO Always TypeError */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "user-supplied statement class must be derived from PDOStatement"); - PDO_HANDLE_DBH_ERR(); - RETURN_FALSE; + zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement"); + RETURN_THROWS(); } if (dbstmt_ce->constructor && !(dbstmt_ce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) { - /* TODO Always TError */ + /* TODO Always Error? */ pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement class cannot have a public constructor"); PDO_HANDLE_DBH_ERR(); RETURN_FALSE; } - if ((item = zend_hash_index_find(Z_ARRVAL_P(opt), 1)) != NULL) { + if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 1)) != NULL) { if (Z_TYPE_P(item) != IS_ARRAY) { - /* TODO Always TypeError */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "PDO::ATTR_STATEMENT_CLASS requires format array(classname, ctor_args); " - "ctor_args must be an array" - ); - PDO_HANDLE_DBH_ERR(); - RETURN_FALSE; + zend_type_error("PDO::ATTR_STATEMENT_CLASS's ctor_args must be ?array, %s given", + zend_zval_type_name(value)); + RETURN_THROWS(); } ZVAL_COPY_VALUE(&ctor_args, item); } else { From e711702ecb540613872bf83936f68fd0899b1697 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 17:02:36 +0100 Subject: [PATCH 19/50] Drop superflous PDO_STMT_CLEAR_ERR(); --- ext/pdo/pdo_stmt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 718f886909d8b..4ce148bc9fa33 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1749,7 +1749,6 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l flags = fetch_mode & PDO_FETCH_FLAGS; if (!pdo_stmt_verify_mode(stmt, fetch_mode, mode_arg_num, false)) { - PDO_STMT_CLEAR_ERR(); return false; } From b2a0a2efe2afeb3f34fe23e260a3bb4b47a0ab2d Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Fri, 25 Sep 2020 17:32:23 +0100 Subject: [PATCH 20/50] Fix NULL pointer access --- ext/pdo/pdo_stmt.c | 4 ++++ ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt | 9 +++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 4ce148bc9fa33..e0b518f640e91 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1347,6 +1347,10 @@ PHP_METHOD(PDOStatement, fetchAll) ZSTR_VAL(get_active_function_or_method_name()), ZEND_NUM_ARGS()); RETURN_THROWS(); } + if (arg2 == NULL) { + zend_argument_type_error(2, "must be a callable, null given"); + RETURN_THROWS(); + } /* TODO Check it is a callable? */ ZVAL_COPY_VALUE(&stmt->fetch.func.function, arg2); if (do_fetch_func_prepare(stmt) == false) { diff --git a/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt b/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt index 3910d8cc4dd69..a44e4fd495beb 100644 --- a/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt +++ b/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt @@ -116,7 +116,7 @@ object(PDOStatement)#%d (1) { ["queryString"]=> string(21) "SELECT * FROM testing" } -data: 2, +data: 2, array(2) { [0]=> string(3) "PHP" @@ -125,11 +125,8 @@ array(2) { } function "nothing" not found or invalid function name function "" not found or invalid function name +PDOStatement::fetchAll(): Argument #2 ($arg2) must be a callable, null given no array or string given -no array or string given - -Warning: PDOStatement::fetchAll(): SQLSTATE[HY000]: General error in %s on line %d -bool(false) cannot access "self" when no class scope is active array(2) { [0]=> @@ -145,4 +142,4 @@ array(2) { } non-static method bar::test2() cannot be called statically non-static method bar::test3() cannot be called statically -class bar does not have a method "inexistent" in %s on line %d +class bar does not have a method "inexistent" From d5d51e814336330f41bfe6fa4857afde3f3e5934 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 01:43:24 +0100 Subject: [PATCH 21/50] Split bug 44159 SQLite difference in behaviour to a dedicated variant --- ext/pdo/tests/bug_44159.phpt | 5 +--- .../tests/bug_44159_sqlite_version.phpt | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt diff --git a/ext/pdo/tests/bug_44159.phpt b/ext/pdo/tests/bug_44159.phpt index 5d0a509b11613..14c3669a09cfb 100644 --- a/ext/pdo/tests/bug_44159.phpt +++ b/ext/pdo/tests/bug_44159.phpt @@ -15,7 +15,7 @@ require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc'; $pdo = PDOTest::factory(); $pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING); -$attrs = array(PDO::ATTR_STATEMENT_CLASS, PDO::ATTR_STRINGIFY_FETCHES, PDO::NULL_TO_STRING); +$attrs = array(PDO::ATTR_STATEMENT_CLASS, PDO::ATTR_STRINGIFY_FETCHES); foreach ($attrs as $attr) { try { @@ -45,6 +45,3 @@ TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given TypeError: Attribute value must be int for selected attribute, null given bool(true) bool(true) -bool(false) -bool(false) -bool(false) diff --git a/ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt b/ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt new file mode 100644 index 0000000000000..ced86346d5c54 --- /dev/null +++ b/ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt @@ -0,0 +1,27 @@ +--TEST-- +PDO Common: Bug #44159 (Crash: $pdo->setAttribute(PDO::STATEMENT_ATTR_CLASS, NULL)): SQLite variant +--SKIPIF-- + +--FILE-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING); + +var_dump($pdo->setAttribute(PDO::NULL_TO_STRING, NULL)); +var_dump($pdo->setAttribute(PDO::NULL_TO_STRING, 1)); +var_dump($pdo->setAttribute(PDO::NULL_TO_STRING, 'nonsense')); + +@unlink(__DIR__."/foo.db"); + +?> +--EXPECT-- +bool(true) +bool(true) +bool(true) From fb87fcd6c6c2305f0cedfdd16328a34301b444fa Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 02:41:33 +0100 Subject: [PATCH 22/50] Fix memory leaks The zend_string from get_active_function_or_method_name() needs to be released --- ext/pdo/pdo_stmt.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index e0b518f640e91..a7005e4e80c86 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1343,8 +1343,10 @@ PHP_METHOD(PDOStatement, fetchAll) case PDO_FETCH_FUNC: /* Cannot be a default fetch mode */ if (ZEND_NUM_ARGS() != 2) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly 2 argument for PDO::FETCH_FUNC, %d given", - ZSTR_VAL(get_active_function_or_method_name()), ZEND_NUM_ARGS()); + ZSTR_VAL(func), ZEND_NUM_ARGS()); + zend_string_release(func); RETURN_THROWS(); } if (arg2 == NULL) { @@ -1360,8 +1362,10 @@ PHP_METHOD(PDOStatement, fetchAll) case PDO_FETCH_COLUMN: if (ZEND_NUM_ARGS() > 2) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects at most 2 argument for the fetch mode provided, %d given", - ZSTR_VAL(get_active_function_or_method_name()), ZEND_NUM_ARGS()); + ZSTR_VAL(func), ZEND_NUM_ARGS()); + zend_string_release(func); RETURN_THROWS(); } /* Is column index passed? */ @@ -1384,8 +1388,10 @@ PHP_METHOD(PDOStatement, fetchAll) default: /* No support for PDO_FETCH_INTO which takes 2 args??? */ if (ZEND_NUM_ARGS() > 1) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly 1 argument for the fetch mode provided, %d given", - ZSTR_VAL(get_active_function_or_method_name()), ZEND_NUM_ARGS()); + ZSTR_VAL(func), ZEND_NUM_ARGS()); + zend_string_release(func); RETURN_THROWS(); } } @@ -1767,16 +1773,20 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l case PDO_FETCH_NAMED: case PDO_FETCH_KEY_PAIR: if (total_num_args != mode_arg_num) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", - ZSTR_VAL(get_active_function_or_method_name()), mode_arg_num, total_num_args); + ZSTR_VAL(func), mode_arg_num, total_num_args); + zend_string_release(func); return false; } break; case PDO_FETCH_COLUMN: if (total_num_args != arg1_arg_num) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", - ZSTR_VAL(get_active_function_or_method_name()), arg1_arg_num, total_num_args); + ZSTR_VAL(func), arg1_arg_num, total_num_args); + zend_string_release(func); return false; } if (Z_TYPE_P(arg1) != IS_LONG) { @@ -1796,8 +1806,10 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l /* Gets its class name from 1st column */ if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) { if (arg1 || constructor_args) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", - ZSTR_VAL(get_active_function_or_method_name()), mode_arg_num, total_num_args); + ZSTR_VAL(func), mode_arg_num, total_num_args); + zend_string_release(func); return false; } stmt->fetch.cls.ce = NULL; @@ -1805,8 +1817,10 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l zend_class_entry *cep; if (!arg1 || total_num_args > constructor_arg_num) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", - ZSTR_VAL(get_active_function_or_method_name()), constructor_arg_num, total_num_args); + ZSTR_VAL(func), constructor_arg_num, total_num_args); + zend_string_release(func); return false; } if (Z_TYPE_P(arg1) != IS_STRING) { @@ -1829,8 +1843,10 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l case PDO_FETCH_INTO: if (total_num_args != arg1_arg_num) { + zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", - ZSTR_VAL(get_active_function_or_method_name()), arg1_arg_num, total_num_args); + ZSTR_VAL(func), arg1_arg_num, total_num_args); + zend_string_release(func); return false; } if (Z_TYPE_P(arg1) != IS_OBJECT) { @@ -1886,7 +1902,7 @@ PHP_METHOD(PDOStatement, setFetchMode) /* {{{ Advances to the next rowset in a multi-rowset statement handle. Returns true if it succeeded, false otherwise */ -static int pdo_stmt_do_next_rowset(pdo_stmt_t *stmt) +static bool pdo_stmt_do_next_rowset(pdo_stmt_t *stmt) { /* un-describe */ if (stmt->columns) { From d95763d66cc800329f1b72a5a256c82f928d7a10 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 17:09:52 +0100 Subject: [PATCH 23/50] Make unintialized PDO object an Error --- ext/pdo/php_pdo.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/pdo/php_pdo.h b/ext/pdo/php_pdo.h index 9d9e73915dfc6..89cd22c8e8baf 100644 --- a/ext/pdo/php_pdo.h +++ b/ext/pdo/php_pdo.h @@ -53,11 +53,11 @@ PHP_MINFO_FUNCTION(pdo); #define REGISTER_PDO_CLASS_CONST_STRING(const_name, value) \ zend_declare_class_constant_stringl(php_pdo_get_dbh_ce(), const_name, sizeof(const_name)-1, value, sizeof(value)-1); -#define PDO_CONSTRUCT_CHECK \ - if (!dbh->driver) { \ - pdo_raise_impl_error(dbh, NULL, "00000", "PDO constructor was not called"); \ - return; \ - } \ +#define PDO_CONSTRUCT_CHECK \ + if (!dbh->driver) { \ + zend_throw_error(NULL, "PDO object is not initialized, constructor was not called"); \ + RETURN_THROWS(); \ + } \ #endif /* PHP_PDO_H */ From dc3af249b7ef2a011e1a37982439ef28752bd33c Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 17:19:38 +0100 Subject: [PATCH 24/50] Drop string type as acceptable type when checking for integer --- ext/pdo/pdo_dbh.c | 3 +-- ext/pdo/tests/bug_44159.phpt | 2 +- ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt | 1 + ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt | 1 + 4 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index bdfa098546554..c527de1bc639f 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -678,9 +678,8 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* { zend_long lval; -/* TODO Always TypeError (also restrict to actual integer?) */ #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) { \ + if (Z_TYPE_P(value) != IS_LONG && Z_TYPE_P(value) != IS_FALSE && Z_TYPE_P(value) != IS_TRUE) { \ zend_type_error("Attribute value must be int for selected attribute, %s given", zend_zval_type_name(value)); \ return FAILURE; \ } \ diff --git a/ext/pdo/tests/bug_44159.phpt b/ext/pdo/tests/bug_44159.phpt index 14c3669a09cfb..b63dd5b77eaa9 100644 --- a/ext/pdo/tests/bug_44159.phpt +++ b/ext/pdo/tests/bug_44159.phpt @@ -44,4 +44,4 @@ TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, int given TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given TypeError: Attribute value must be int for selected attribute, null given bool(true) -bool(true) +TypeError: Attribute value must be int for selected attribute, string given diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt index 72a6004e50852..0f0f585f09772 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt @@ -160,6 +160,7 @@ error_reporting=E_ALL --EXPECTF-- TypeError: Attribute value must be int for selected attribute, array given TypeError: Attribute value must be int for selected attribute, stdClass given +TypeError: Attribute value must be int for selected attribute, string given ValueError: Error mode must be one of 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 diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt index 192c6eb1f979e..61ee29d4d98c8 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt @@ -90,6 +90,7 @@ MySQLPDOTest::skip(); --EXPECTF-- Attribute value must be int for selected attribute, array given Attribute value must be int for selected attribute, stdClass given +Attribute value must be int for selected attribute, string given array(1) { [0]=> array(6) { From df49654c104df70b622fd802910a97e09db36750 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 17:26:48 +0100 Subject: [PATCH 25/50] Uncodnitional error for attempting to write/delete read-only property --- ext/pdo/pdo_stmt.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index a7005e4e80c86..a6a70244272cc 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -2058,11 +2058,8 @@ PHP_METHOD(PDOStatement, getIterator) /* {{{ overloaded handlers for PDOStatement class */ static zval *dbstmt_prop_write(zend_object *object, zend_string *name, zval *value, void **cache_slot) { - pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); - if (strcmp(ZSTR_VAL(name), "queryString") == 0) { - /* TODO Error? */ - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "property queryString is read only"); + zend_throw_error(NULL, "Property queryString is read only"); return value; } else { return zend_std_write_property(object, name, value, cache_slot); @@ -2071,11 +2068,8 @@ static zval *dbstmt_prop_write(zend_object *object, zend_string *name, zval *val static void dbstmt_prop_delete(zend_object *object, zend_string *name, void **cache_slot) { - pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); - if (strcmp(ZSTR_VAL(name), "queryString") == 0) { - /* TODO Error? */ - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "property queryString is read only"); + zend_throw_error(NULL, "Property queryString is read only"); } else { zend_std_unset_property(object, name, cache_slot); } From 8c1f0ffd89969cf84aac44a3e593c4d3d24b3207 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 17:37:17 +0100 Subject: [PATCH 26/50] Tiny Refactor --- ext/pdo/pdo_stmt.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index a6a70244272cc..ec3ff1f339c2b 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1199,10 +1199,10 @@ PHP_METHOD(PDOStatement, fetch) PDO_STMT_CLEAR_ERR(); if (!pdo_stmt_verify_mode(stmt, how, 1, false)) { - RETURN_FALSE; + RETURN_THROWS(); } - if (!do_fetch(stmt, return_value, how, ori, off, 0)) { + if (!do_fetch(stmt, return_value, how, ori, off, NULL)) { PDO_HANDLE_STMT_ERR(); RETURN_FALSE; } @@ -1212,8 +1212,6 @@ PHP_METHOD(PDOStatement, fetch) /* {{{ Fetches the next row and returns it as an object. */ PHP_METHOD(PDOStatement, fetchObject) { - zend_long ori = PDO_FETCH_ORI_NEXT; - zend_long off = 0; zend_class_entry *ce = NULL; zend_class_entry *old_ce; zval old_ctor_args, *ctor_args = NULL; @@ -1229,7 +1227,7 @@ PHP_METHOD(PDOStatement, fetchObject) PDO_STMT_CLEAR_ERR(); if (!pdo_stmt_verify_mode(stmt, PDO_FETCH_CLASS, 0, false)) { - RETURN_FALSE; + RETURN_THROWS(); } old_ce = stmt->fetch.cls.ce; @@ -1251,7 +1249,7 @@ PHP_METHOD(PDOStatement, fetchObject) stmt->fetch.cls.ce = zend_standard_class_def; } - if (!do_fetch(stmt, return_value, PDO_FETCH_CLASS, ori, off, 0)) { + if (!do_fetch(stmt, return_value, PDO_FETCH_CLASS, PDO_FETCH_ORI_NEXT, /* offset */ 0, NULL)) { PDO_HANDLE_STMT_ERR(); RETVAL_FALSE; } @@ -1304,7 +1302,7 @@ PHP_METHOD(PDOStatement, fetchAll) PHP_STMT_GET_OBJ; if (!pdo_stmt_verify_mode(stmt, how, 1, true)) { - RETURN_FALSE; + RETURN_THROWS(); } old_ce = stmt->fetch.cls.ce; @@ -1411,22 +1409,22 @@ PHP_METHOD(PDOStatement, fetchAll) array_init(return_value); return_all = return_value; } else { - return_all = 0; + return_all = NULL; } - if (!do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, 0, return_all)) { + if (!do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, /* offset */ 0, return_all)) { error = 2; } } if (!error) { if ((how & PDO_FETCH_GROUP)) { - while (do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, 0, return_all)); + while (do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, /* offset */ 0, return_all)); } else if (how == PDO_FETCH_KEY_PAIR || (how == PDO_FETCH_USE_DEFAULT && stmt->default_fetch_type == PDO_FETCH_KEY_PAIR)) { - while (do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, 0, return_all)); + while (do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, /* offset */ 0, return_all)); } else { array_init(return_value); do { zend_hash_next_index_insert_new(Z_ARRVAL_P(return_value), &data); - } while (do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, 0, 0)); + } while (do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, /* offset */ 0, NULL)); } } @@ -1863,16 +1861,6 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l stmt->default_fetch_type = fetch_mode; - /* TODO Is this still needed? */ - /* - * PDO error (if any) has already been raised at this point. - * - * The error_code is cleared, otherwise the caller will read the - * last error message from the driver. - * - */ - PDO_STMT_CLEAR_ERR(); - return true; } @@ -2255,7 +2243,7 @@ static void pdo_stmt_iter_move_forwards(zend_object_iterator *iter) } if (!do_fetch(stmt, &I->fetch_ahead, PDO_FETCH_USE_DEFAULT, - PDO_FETCH_ORI_NEXT, 0, 0)) { + PDO_FETCH_ORI_NEXT, /* offset */ 0, NULL)) { PDO_HANDLE_STMT_ERR(); I->key = (zend_ulong)-1; @@ -2295,7 +2283,7 @@ zend_object_iterator *pdo_stmt_iter_get(zend_class_entry *ce, zval *object, int ZVAL_OBJ(&I->iter.data, Z_OBJ_P(object)); if (!do_fetch(stmt, &I->fetch_ahead, PDO_FETCH_USE_DEFAULT, - PDO_FETCH_ORI_NEXT, 0, 0)) { + PDO_FETCH_ORI_NEXT, /* offset */ 0, NULL)) { PDO_HANDLE_STMT_ERR(); I->key = (zend_ulong)-1; ZVAL_UNDEF(&I->fetch_ahead); From 2f2f5ba942df8a28d4ee433a67e45ed1fee2c629 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 17:40:16 +0100 Subject: [PATCH 27/50] Make a check an assertion --- ext/pdo/pdo_stmt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index ec3ff1f339c2b..43c2d70570b4c 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1226,9 +1226,8 @@ PHP_METHOD(PDOStatement, fetchObject) PHP_STMT_GET_OBJ; PDO_STMT_CLEAR_ERR(); - if (!pdo_stmt_verify_mode(stmt, PDO_FETCH_CLASS, 0, false)) { - RETURN_THROWS(); - } + /* use pdo_stmt_verify_mode() to set fetch mode for this specific statement */ + ZEND_ASSERT(pdo_stmt_verify_mode(stmt, PDO_FETCH_CLASS, 0, false)); old_ce = stmt->fetch.cls.ce; ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args); From 2b57e465a89c0efc6b7d4c7e216f8f3507450e04 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 17:47:49 +0100 Subject: [PATCH 28/50] Make unitialized object an error --- ext/pdo/pdo_stmt.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 43c2d70570b4c..fdf85d802d6d0 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -34,11 +34,12 @@ #include "php_memory_streams.h" #include "pdo_stmt_arginfo.h" -#define PHP_STMT_GET_OBJ \ - pdo_stmt_t *stmt = Z_PDO_STMT_P(ZEND_THIS); \ - if (!stmt->dbh) { \ - RETURN_FALSE; \ - } \ +#define PHP_STMT_GET_OBJ \ + pdo_stmt_t *stmt = Z_PDO_STMT_P(ZEND_THIS); \ + if (!stmt->dbh) { \ + zend_throw_error(NULL, "PDO Object is uninitialized"); \ + RETURN_THROWS(); \ + } \ static inline int rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_param_data *param) /* {{{ */ { From 095513b93f83d1bab0a3377f8b03123b2a7606c3 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:01:12 +0100 Subject: [PATCH 29/50] Refactor PDOStatement::fetchAll() Parts can be simplified as we throw early --- ext/pdo/pdo_stmt.c | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index fdf85d802d6d0..16c690cacb759 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1287,11 +1287,12 @@ PHP_METHOD(PDOStatement, fetchColumn) PHP_METHOD(PDOStatement, fetchAll) { zend_long how = PDO_FETCH_USE_DEFAULT; - zval data, *return_all; + zval data, *return_all = NULL; zval *arg2 = NULL; zend_class_entry *old_ce; zval old_ctor_args, *ctor_args = NULL; - int error = 0, flags, old_arg_count; + bool error = false; + int flags, old_arg_count; ZEND_PARSE_PARAMETERS_START(0, 3) Z_PARAM_OPTIONAL @@ -1401,20 +1402,17 @@ PHP_METHOD(PDOStatement, fetchAll) how |= stmt->default_fetch_type & ~PDO_FETCH_FLAGS; } - if (!error) { - PDO_STMT_CLEAR_ERR(); - if ((how & PDO_FETCH_GROUP) || how == PDO_FETCH_KEY_PAIR || - (how == PDO_FETCH_USE_DEFAULT && stmt->default_fetch_type == PDO_FETCH_KEY_PAIR) - ) { - array_init(return_value); - return_all = return_value; - } else { - return_all = NULL; - } - if (!do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, /* offset */ 0, return_all)) { - error = 2; - } + PDO_STMT_CLEAR_ERR(); + if ((how & PDO_FETCH_GROUP) || how == PDO_FETCH_KEY_PAIR || + (how == PDO_FETCH_USE_DEFAULT && stmt->default_fetch_type == PDO_FETCH_KEY_PAIR) + ) { + array_init(return_value); + return_all = return_value; + } + if (!do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, /* offset */ 0, return_all)) { + error = true; } + if (!error) { if ((how & PDO_FETCH_GROUP)) { while (do_fetch(stmt, &data, how | flags, PDO_FETCH_ORI_NEXT, /* offset */ 0, return_all)); @@ -1430,19 +1428,16 @@ PHP_METHOD(PDOStatement, fetchAll) do_fetch_opt_finish(stmt, 0); + /* Restore defaults which were changed by PDO_FETCH_CLASS mode */ stmt->fetch.cls.ce = old_ce; ZVAL_COPY_VALUE(&stmt->fetch.cls.ctor_args, &old_ctor_args); stmt->fetch.cls.fci.param_count = old_arg_count; + /* on no results, return an empty array */ if (error) { PDO_HANDLE_STMT_ERR(); - if (error != 2) { - RETURN_FALSE; - } else { /* on no results, return an empty array */ - if (Z_TYPE_P(return_value) != IS_ARRAY) { - array_init(return_value); - } - return; + if (Z_TYPE_P(return_value) != IS_ARRAY) { + array_init(return_value); } } } From d56559f108822b908a144670e00316574b67441d Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:01:58 +0100 Subject: [PATCH 30/50] Fix stubs --- ext/pdo/pdo_stmt.stub.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/pdo/pdo_stmt.stub.php b/ext/pdo/pdo_stmt.stub.php index a170c21b1e744..64164ba6e5d2a 100644 --- a/ext/pdo/pdo_stmt.stub.php +++ b/ext/pdo/pdo_stmt.stub.php @@ -34,7 +34,7 @@ public function execute(?array $input_parameters = null) {} /** @return mixed */ public function fetch(int $fetch_style = PDO::FETCH_BOTH, int $cursor_orientation = PDO::FETCH_ORI_NEXT, int $cursor_offset = 0) {} - /** @return array|false */ + /** @return array */ public function fetchAll(int $fetch_style = PDO::FETCH_BOTH, mixed $arg2 = null, ?array $constructorArguments = []) {} /** @return mixed */ From 66f35b52371929bbbb9b6e6b11426c02279787eb Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:05:36 +0100 Subject: [PATCH 31/50] Refactor PDOStatement::setAttribute() to be more explicit --- ext/pdo/pdo_stmt.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 16c690cacb759..506157480bf41 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1609,8 +1609,11 @@ PHP_METHOD(PDOStatement, setAttribute) ZEND_PARSE_PARAMETERS_END(); PHP_STMT_GET_OBJ; + + /* Driver hasn't registered a function for setting attributes */ if (!stmt->methods->set_attribute) { - goto fail; + pdo_raise_impl_error(stmt->dbh, stmt, "IM001", "This driver doesn't support setting attributes"); + RETURN_FALSE; } PDO_STMT_CLEAR_ERR(); @@ -1618,12 +1621,8 @@ PHP_METHOD(PDOStatement, setAttribute) RETURN_TRUE; } -fail: - if (!stmt->methods->set_attribute) { - pdo_raise_impl_error(stmt->dbh, stmt, "IM001", "This driver doesn't support setting attributes"); - } else { - PDO_HANDLE_STMT_ERR(); - } + /* Error while setting attribute */ + PDO_HANDLE_STMT_ERR(); RETURN_FALSE; } /* }}} */ From 7716b916fe95d5d955642a26bcc9d1fc6311ccfb Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:12:06 +0100 Subject: [PATCH 32/50] Add a TODO comment --- ext/pdo/pdo_stmt.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 506157480bf41..10321a4481ed8 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1979,6 +1979,8 @@ PHP_METHOD(PDOStatement, debugDumpParams) ZEND_PARSE_PARAMETERS_NONE(); PHP_STMT_GET_OBJ; + + /* TODO: Change to assertion? */ if (out == NULL) { RETURN_FALSE; } From 8e9fa1539b388a60289e1ea641f6cecd34e133ef Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:20:07 +0100 Subject: [PATCH 33/50] Redfine todo comment --- ext/pdo/pdo_stmt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 10321a4481ed8..2c401daf61894 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -872,8 +872,9 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h zval_ptr_dtor_str(&val); } ce = stmt->fetch.cls.ce; + /* TODO: Make this an assertion and ensure this is true higher up? */ if (!ce) { - /* TODO ArgumentCountError? */ + /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch class specified"); return 0; } From 1725e31860c75d6801b8a6e62188f738b727c4c4 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:27:46 +0100 Subject: [PATCH 34/50] Introduce ValueError for empty bind param names --- ext/pdo/pdo_stmt.c | 8 ++++++++ ext/pdo/pdo_stmt_arginfo.h | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 2c401daf61894..3e3961a8bf8c8 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1466,6 +1466,10 @@ static void register_bound_param(INTERNAL_FUNCTION_PARAMETERS, int is_param) /* param.param_type = (int) param_type; if (param.name) { + if (ZSTR_LEN(param.name) == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); + } param.paramno = -1; } else if (param.paramno > 0) { --param.paramno; /* make it zero-based internally */ @@ -1510,6 +1514,10 @@ PHP_METHOD(PDOStatement, bindValue) param.param_type = (int) param_type; if (param.name) { + if (ZSTR_LEN(param.name) == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); + } param.paramno = -1; } else if (param.paramno > 0) { --param.paramno; /* make it zero-based internally */ diff --git a/ext/pdo/pdo_stmt_arginfo.h b/ext/pdo/pdo_stmt_arginfo.h index f0199f7c14cb2..798c66060b6f0 100644 --- a/ext/pdo/pdo_stmt_arginfo.h +++ b/ext/pdo/pdo_stmt_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 6d360d7d412e0ebbbc466f8abf104f3969eae738 */ + * Stub hash: 395813e5ab7b565718430dd17b8da3fc54889eeb */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_bindColumn, 0, 0, 2) ZEND_ARG_TYPE_MASK(0, column, MAY_BE_STRING|MAY_BE_LONG, NULL) From 79e944442f8c79e20dcbe7c69b2e491bb49c69a3 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:31:27 +0100 Subject: [PATCH 35/50] Fix stubs after making uninitialized object return Error --- ext/pdo/pdo_stmt.stub.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/pdo/pdo_stmt.stub.php b/ext/pdo/pdo_stmt.stub.php index 64164ba6e5d2a..bd8125cfde3cf 100644 --- a/ext/pdo/pdo_stmt.stub.php +++ b/ext/pdo/pdo_stmt.stub.php @@ -16,16 +16,16 @@ public function bindValue(string|int $parameter, mixed $value, int $type = PDO:: /** @return bool */ public function closeCursor() {} - /** @return int|false */ + /** @return int */ public function columnCount() {} /** @return bool|null */ public function debugDumpParams() {} - /** @return string|false|null */ + /** @return string|null */ public function errorCode() {} - /** @return array|false */ + /** @return array */ public function errorInfo() {} /** @return bool */ @@ -52,7 +52,7 @@ public function getColumnMeta(int $column) {} /** @return bool */ public function nextRowset() {} - /** @return int|false */ + /** @return int */ public function rowCount() {} /** @return bool */ From 64349d3dc44b179b744ee25c8fd083cc496bb0fd Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 18:50:26 +0100 Subject: [PATCH 36/50] Introduce ValueError for empty string arguments --- ext/pdo/pdo_dbh.c | 38 +++++++++++++++++-- ext/pdo/pdo_stmt_arginfo.h | 2 +- .../tests/pdo_mysql_prepare_emulated.phpt | 9 +++-- .../tests/pdo_mysql_prepare_native.phpt | 9 +++-- 4 files changed, 47 insertions(+), 11 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index c527de1bc639f..5480ab4840c86 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -242,6 +242,11 @@ PHP_METHOD(PDO, __construct) Z_PARAM_ARRAY_OR_NULL(options) ZEND_PARSE_PARAMETERS_END(); + if (username && usernamelen == 0) { + zend_argument_value_error(2, "cannot be empty"); + RETURN_THROWS(); + } + /* parse the data source name */ colon = strchr(data_source, ':'); @@ -497,9 +502,15 @@ PHP_METHOD(PDO, prepare) Z_PARAM_ARRAY(options) ZEND_PARSE_PARAMETERS_END(); - PDO_DBH_CLEAR_ERR(); PDO_CONSTRUCT_CHECK; + if (statement_len == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); + } + + PDO_DBH_CLEAR_ERR(); + if (options && (value = zend_hash_index_find(Z_ARRVAL_P(options), PDO_ATTR_STATEMENT_CLASS)) != NULL) { if (Z_TYPE_P(value) != IS_ARRAY) { zend_type_error("PDO::ATTR_STATEMENT_CLASS's value must be of type array, %s given", @@ -950,8 +961,15 @@ PHP_METHOD(PDO, lastInsertId) Z_PARAM_STRING_OR_NULL(name, namelen) ZEND_PARSE_PARAMETERS_END(); - PDO_DBH_CLEAR_ERR(); PDO_CONSTRUCT_CHECK; + + if (name && namelen == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); + } + + PDO_DBH_CLEAR_ERR(); + if (!dbh->methods->last_id) { pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support lastInsertId()"); RETURN_FALSE; @@ -1060,9 +1078,15 @@ PHP_METHOD(PDO, query) RETURN_THROWS(); } - PDO_DBH_CLEAR_ERR(); PDO_CONSTRUCT_CHECK; + if (statement_len == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); + } + + PDO_DBH_CLEAR_ERR(); + if (!pdo_stmt_instantiate(dbh, return_value, dbh->def_stmt_ce, &dbh->def_stmt_ctor_args)) { if (EXPECTED(!EG(exception))) { pdo_raise_impl_error(dbh, NULL, "HY000", "failed to instantiate user supplied statement class"); @@ -1135,8 +1159,14 @@ PHP_METHOD(PDO, quote) Z_PARAM_LONG(paramtype) ZEND_PARSE_PARAMETERS_END(); - PDO_DBH_CLEAR_ERR(); PDO_CONSTRUCT_CHECK; + + if (str_len == 0) { + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); + } + + PDO_DBH_CLEAR_ERR(); if (!dbh->methods->quoter) { pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support quoting"); RETURN_FALSE; diff --git a/ext/pdo/pdo_stmt_arginfo.h b/ext/pdo/pdo_stmt_arginfo.h index 798c66060b6f0..e0547712f2376 100644 --- a/ext/pdo/pdo_stmt_arginfo.h +++ b/ext/pdo/pdo_stmt_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 395813e5ab7b565718430dd17b8da3fc54889eeb */ + * Stub hash: 12ba83581b1019e2e0c4565e20682b5e83389cae */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_bindColumn, 0, 0, 2) ZEND_ARG_TYPE_MASK(0, column, MAY_BE_STRING|MAY_BE_LONG, NULL) diff --git a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt index 6fe2ff20ba616..6afd57420f9f4 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt @@ -88,9 +88,11 @@ $db = MySQLPDOTest::factory(); if (1 != $db->getAttribute(PDO::MYSQL_ATTR_DIRECT_QUERY)) printf("[002] Unable to switch to emulated prepared statements, test will fail\n"); - // TODO - that's PDO - you can prepare empty statements! - prepex(3, $db, '', - array(), array('execute' => array('sqlstate' => '42000'))); + try { + prepex(3, $db, '', [], ['execute' => ['sqlstate' => '42000']]); + } catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; + } // lets be fair and do the most simple SELECT first $stmt = prepex(4, $db, 'SELECT 1 as "one"'); @@ -328,6 +330,7 @@ $db->exec('DROP TABLE IF EXISTS test'); PDO's PS parser has some problems with invalid SQL and crashes from time to time (check with valgrind...) --EXPECT-- +PDO::prepare(): Argument #1 ($statement) cannot be empty array(1) { ["one"]=> string(1) "1" diff --git a/ext/pdo_mysql/tests/pdo_mysql_prepare_native.phpt b/ext/pdo_mysql/tests/pdo_mysql_prepare_native.phpt index 5ea3e94c1e138..839fc43c15db4 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_prepare_native.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_prepare_native.phpt @@ -99,9 +99,11 @@ $db = MySQLPDOTest::factory(); if (0 != $db->getAttribute(PDO::MYSQL_ATTR_DIRECT_QUERY)) printf("[002] Unable to turn off emulated prepared statements\n"); - // TODO - that's PDO - you can prepare empty statements! - prepex(3, $db, '', - array(), array('prepare' => array('sqlstate' => '42000'))); + try { + prepex(3, $db, '', [], ['prepare' => ['sqlstate' => '42000']]); + } catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; + } // lets be fair and do the most simple SELECT first $stmt = prepex(4, $db, 'SELECT 1 as "one"'); @@ -342,6 +344,7 @@ $db = MySQLPDOTest::factory(); $db->exec('DROP TABLE IF EXISTS test'); ?> --EXPECT-- +PDO::prepare(): Argument #1 ($statement) cannot be empty array(1) { [0]=> array(1) { From a52ce29d8de38871f43155b30300ec700b1a95b2 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 26 Sep 2020 20:40:07 +0100 Subject: [PATCH 37/50] Revert ValueError for empty username --- ext/pdo/pdo_dbh.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 5480ab4840c86..1cf3fdfbe719a 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -242,11 +242,6 @@ PHP_METHOD(PDO, __construct) Z_PARAM_ARRAY_OR_NULL(options) ZEND_PARSE_PARAMETERS_END(); - if (username && usernamelen == 0) { - zend_argument_value_error(2, "cannot be empty"); - RETURN_THROWS(); - } - /* parse the data source name */ colon = strchr(data_source, ':'); From d94521b8a4a1c1b6fbd9dc31bd59d1e99413a503 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sun, 27 Sep 2020 23:36:36 +0100 Subject: [PATCH 38/50] Make fetch argument variadic --- ext/pdo/pdo_dbh.stub.php | 2 +- ext/pdo/pdo_dbh_arginfo.h | 5 ++--- ext/pdo/pdo_stmt.stub.php | 4 ++-- ext/pdo/pdo_stmt_arginfo.h | 8 +++----- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/ext/pdo/pdo_dbh.stub.php b/ext/pdo/pdo_dbh.stub.php index ba33baf59e8ce..2348abf048d86 100644 --- a/ext/pdo/pdo_dbh.stub.php +++ b/ext/pdo/pdo_dbh.stub.php @@ -37,7 +37,7 @@ public function lastInsertId(?string $name = null) {} public function prepare(string $statement, array $driver_options = []) {} /** @return PDOStatement|false */ - public function query(string $statement, ?int $fetch_mode = null, mixed $arg1 = null, ?array $constructorArguments = []) {} + public function query(string $statement, ?int $fetch_mode = null, mixed ...$fetch_mode_args) {} /** @return string|false */ public function quote(string $string, int $parameter_type = PDO::PARAM_STR) {} diff --git a/ext/pdo/pdo_dbh_arginfo.h b/ext/pdo/pdo_dbh_arginfo.h index 179821bf20583..a3ab1994ed67d 100644 --- a/ext/pdo/pdo_dbh_arginfo.h +++ b/ext/pdo/pdo_dbh_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 1374ebd53913af744634fcbd745f0182d87fff1a */ + * Stub hash: 0c7acc78768ad1fb77a09a24ebd4d4e660b350c9 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDO___construct, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, dsn, IS_STRING, 0) @@ -41,8 +41,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDO_query, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, statement, IS_STRING, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, fetch_mode, IS_LONG, 1, "null") - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, arg1, IS_MIXED, 0, "null") - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, constructorArguments, IS_ARRAY, 1, "[]") + ZEND_ARG_VARIADIC_TYPE_INFO(0, fetch_mode_args, IS_MIXED, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDO_quote, 0, 0, 1) diff --git a/ext/pdo/pdo_stmt.stub.php b/ext/pdo/pdo_stmt.stub.php index bd8125cfde3cf..2732c8a71da56 100644 --- a/ext/pdo/pdo_stmt.stub.php +++ b/ext/pdo/pdo_stmt.stub.php @@ -35,7 +35,7 @@ public function execute(?array $input_parameters = null) {} public function fetch(int $fetch_style = PDO::FETCH_BOTH, int $cursor_orientation = PDO::FETCH_ORI_NEXT, int $cursor_offset = 0) {} /** @return array */ - public function fetchAll(int $fetch_style = PDO::FETCH_BOTH, mixed $arg2 = null, ?array $constructorArguments = []) {} + public function fetchAll(int $fetch_style = PDO::FETCH_BOTH, mixed ...$fetch_mode_args) {} /** @return mixed */ public function fetchColumn(int $column_number = 0) {} @@ -59,7 +59,7 @@ public function rowCount() {} public function setAttribute(int $attribute, mixed $value) {} /** @return bool */ - public function setFetchMode(int $mode, mixed $arg1 = null, ?array $constructorArguments = []) {} + public function setFetchMode(int $mode, mixed ...$fetch_mode_args) {} public function getIterator(): Iterator {} } diff --git a/ext/pdo/pdo_stmt_arginfo.h b/ext/pdo/pdo_stmt_arginfo.h index e0547712f2376..157507a852ded 100644 --- a/ext/pdo/pdo_stmt_arginfo.h +++ b/ext/pdo/pdo_stmt_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 12ba83581b1019e2e0c4565e20682b5e83389cae */ + * Stub hash: c12bc1c5d1e3dbd8cce67e50c974b20ec5564e67 */ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_bindColumn, 0, 0, 2) ZEND_ARG_TYPE_MASK(0, column, MAY_BE_STRING|MAY_BE_LONG, NULL) @@ -46,8 +46,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_fetchAll, 0, 0, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, fetch_style, IS_LONG, 0, "PDO::FETCH_BOTH") - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, arg2, IS_MIXED, 0, "null") - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, constructorArguments, IS_ARRAY, 1, "[]") + ZEND_ARG_VARIADIC_TYPE_INFO(0, fetch_mode_args, IS_MIXED, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_fetchColumn, 0, 0, 0) @@ -78,8 +77,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_PDOStatement_setFetchMode, 0, 0, 1) ZEND_ARG_TYPE_INFO(0, mode, IS_LONG, 0) - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, arg1, IS_MIXED, 0, "null") - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, constructorArguments, IS_ARRAY, 1, "[]") + ZEND_ARG_VARIADIC_TYPE_INFO(0, fetch_mode_args, IS_MIXED, 0) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class_PDOStatement_getIterator, 0, 0, Iterator, 0) From ee37a1d45a5b5cb8d0fc786e9d7478d74cbf824f Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 02:58:30 +0100 Subject: [PATCH 39/50] Fix test after making fetch args variadic --- ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt b/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt index a44e4fd495beb..814a01a6472d6 100644 --- a/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt +++ b/ext/pdo_sqlite/tests/pdo_fetch_func_001.phpt @@ -125,7 +125,7 @@ array(2) { } function "nothing" not found or invalid function name function "" not found or invalid function name -PDOStatement::fetchAll(): Argument #2 ($arg2) must be a callable, null given +PDOStatement::fetchAll(): Argument #2 must be a callable, null given no array or string given cannot access "self" when no class scope is active array(2) { From 89b233c55457e53ee7d30253e74904767a92560b Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 11:07:42 +0100 Subject: [PATCH 40/50] Review for pdo_dbh.c Promote a couple of confirmed cases to unconditional error Fix error messages --- ext/pdo/pdo_dbh.c | 51 ++++++++----------- ext/pdo/tests/bug_44159.phpt | 6 +-- .../tests/pdo_mysql_attr_errmode.phpt | 2 +- .../tests/pdo_mysql_attr_statement_class.phpt | 13 ++--- 4 files changed, 30 insertions(+), 42 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 1cf3fdfbe719a..3e24d125800ab 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -428,8 +428,7 @@ static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry /* This implies an error within PDO if this does not hold */ ZEND_ASSERT(Z_TYPE_P(ctor_args) == IS_ARRAY); if (!dbstmt_ce->constructor) { - /* TODO Error? */ - pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement does not accept constructor arguments"); + zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments"); return NULL; } } @@ -508,34 +507,31 @@ PHP_METHOD(PDO, prepare) if (options && (value = zend_hash_index_find(Z_ARRVAL_P(options), PDO_ATTR_STATEMENT_CLASS)) != NULL) { if (Z_TYPE_P(value) != IS_ARRAY) { - zend_type_error("PDO::ATTR_STATEMENT_CLASS's value must be of type array, %s given", + zend_type_error("PDO::ATTR_STATEMENT_CLASS value must be of type array, %s given", zend_zval_type_name(value)); RETURN_THROWS(); } if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL) { - zend_value_error("PDO::ATTR_STATEMENT_CLASS's value must be an array with the following " - "format array(classname, array(ctor_args))"); + zend_value_error("PDO::ATTR_STATEMENT_CLASS value must be an array with the format " + "array(classname, array(ctor_args))"); RETURN_THROWS(); } if (Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL) { - zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be a valid class"); + zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be a valid class"); RETURN_THROWS(); } dbstmt_ce = pce; if (!instanceof_function(dbstmt_ce, pdo_dbstmt_ce)) { - zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement"); + zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement"); RETURN_THROWS(); } if (dbstmt_ce->constructor && !(dbstmt_ce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) { - /* TODO Always Error? */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "user-supplied statement class cannot have a public constructor"); - PDO_HANDLE_DBH_ERR(); - RETURN_FALSE; + zend_type_error("User-supplied statement class cannot have a public constructor"); + RETURN_THROWS(); } 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's ctor_args must be ?array, %s given", + zend_type_error("PDO::ATTR_STATEMENT_CLASS ctor_args must be ?array, %s given", zend_zval_type_name(value)); RETURN_THROWS(); } @@ -548,12 +544,10 @@ PHP_METHOD(PDO, prepare) ZVAL_COPY_VALUE(&ctor_args, &dbh->def_stmt_ctor_args); } + /* Need to check if pdo_stmt_instantiate() throws an exception unconditionally to see if can change the RETURN_FALSE; */ if (!pdo_stmt_instantiate(dbh, return_value, dbstmt_ce, &ctor_args)) { if (EXPECTED(!EG(exception))) { - /* TODO Error? */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "failed to instantiate user-supplied statement class" - ); + zend_throw_error(NULL, "Cannot instantiate user-supplied statement class"); } PDO_HANDLE_DBH_ERR(); RETURN_FALSE; @@ -701,7 +695,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* dbh->error_mode = lval; return SUCCESS; default: - zend_value_error("Error mode must be one of PDO::ERRMODE_* constants"); + zend_value_error("Error mode must be one of the PDO::ERRMODE_* constants"); return FAILURE; } return FAILURE; @@ -716,7 +710,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* dbh->desired_case = lval; return SUCCESS; default: - zend_value_error("Case folding mode must be one of PDO::CASE_* constants"); + zend_value_error("Case folding mode must be one of the PDO::CASE_* constants"); return FAILURE; } return FAILURE; @@ -731,7 +725,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* 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 default fetch mode"); + zend_value_error("PDO::FETCH_INTO and PDO::FETCH_CLASS cannot be set as the default fetch mode"); return FAILURE; } } @@ -764,28 +758,25 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* return FAILURE; } if (Z_TYPE_P(value) != IS_ARRAY) { - zend_type_error("PDO::ATTR_STATEMENT_CLASS's value must be of type array, %s given", + zend_type_error("PDO::ATTR_STATEMENT_CLASS value must be of type array, %s given", zend_zval_type_name(value)); return FAILURE; } if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL) { - zend_value_error("PDO::ATTR_STATEMENT_CLASS's value must be an array with the following " - "format array(classname, array(ctor_args))"); + zend_value_error("PDO::ATTR_STATEMENT_CLASS value must be an array with the format " + "array(classname, array(ctor_args))"); return FAILURE; } if (Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL) { - zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be a valid class"); + zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be a valid class"); return FAILURE; } if (!instanceof_function(pce, pdo_dbstmt_ce)) { - zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement"); + zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement"); return FAILURE; } if (pce->constructor && !(pce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) { - /* TODO Always Error? */ - pdo_raise_impl_error(dbh, NULL, "HY000", - "user-supplied statement class cannot have a public constructor"); - PDO_HANDLE_DBH_ERR(); + zend_type_error("User-supplied statement class cannot have a public constructor"); return FAILURE; } dbh->def_stmt_ce = pce; @@ -795,7 +786,7 @@ static int 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's ctor_args must be ?array, %s given", + zend_type_error("PDO::ATTR_STATEMENT_CLASS ctor_args must be ?array, %s given", zend_zval_type_name(value)); return FAILURE; } diff --git a/ext/pdo/tests/bug_44159.phpt b/ext/pdo/tests/bug_44159.phpt index b63dd5b77eaa9..af212ed555e2f 100644 --- a/ext/pdo/tests/bug_44159.phpt +++ b/ext/pdo/tests/bug_44159.phpt @@ -39,9 +39,9 @@ foreach ($attrs as $attr) { ?> --EXPECT-- -TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, null given -TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, int given -TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given +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 int for selected attribute, null given bool(true) TypeError: Attribute value must be int for selected attribute, string given diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt index 0f0f585f09772..da30f62f27b10 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt @@ -161,7 +161,7 @@ error_reporting=E_ALL TypeError: Attribute value must be int for selected attribute, array given TypeError: Attribute value must be int for selected attribute, stdClass given TypeError: Attribute value must be int for selected attribute, string given -ValueError: Error mode must be one of PDO::ERRMODE_* constants +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 diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt index d4d64c5eb3c11..6d536f9deddd2 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt @@ -125,14 +125,11 @@ array(1) { [0]=> string(12) "PDOStatement" } -PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given -PDO::ATTR_STATEMENT_CLASS's class must be a valid class -PDO::ATTR_STATEMENT_CLASS's class must be a valid class -PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: user-supplied statement class cannot have a public constructor in %s on line %d - -Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d +PDO::ATTR_STATEMENT_CLASS value must be of type array, string given +PDO::ATTR_STATEMENT_CLASS class must be a valid class +PDO::ATTR_STATEMENT_CLASS class must be a valid class +PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement +TypeError: User-supplied statement class cannot have a public constructor array(2) { [0]=> string(12) "mystatement4" From 978b43dd85ae1bc7024f896c12071183dfb59049 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 11:08:33 +0100 Subject: [PATCH 41/50] Drop todo comments in pdo_sql_parser.re --- ext/pdo/pdo_sql_parser.re | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ext/pdo/pdo_sql_parser.re b/ext/pdo/pdo_sql_parser.re index 1a132cb7b22fb..ee0ce7168d384 100644 --- a/ext/pdo/pdo_sql_parser.re +++ b/ext/pdo/pdo_sql_parser.re @@ -151,7 +151,6 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, const char *inquery, size_t inque /* did the query make sense to me? */ if (query_type == (PDO_PLACEHOLDER_NAMED|PDO_PLACEHOLDER_POSITIONAL)) { /* they mixed both types; punt */ - /* TODO Always Error */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "mixed named and positional parameters"); ret = -1; goto clean_up; @@ -193,7 +192,6 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, const char *inquery, size_t inque goto safe; } } - /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "number of bound variables does not match number of tokens"); ret = -1; goto clean_up; @@ -224,7 +222,6 @@ safe: if (param == NULL) { /* parameter was not defined */ ret = -1; - /* TODO Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "parameter was not defined"); goto clean_up; } @@ -260,7 +257,6 @@ safe: zend_string_release_ex(buf, 0); } } else { - /* TODO Always TypeError */ pdo_raise_impl_error(stmt->dbh, stmt, "HY105", "Expected a stream resource"); ret = -1; goto clean_up; From 909651e2bc543373ad5db5893a4b1f379d406f35 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 11:16:06 +0100 Subject: [PATCH 42/50] Revert + add test for empty string in PDO::qutoe() --- ext/pdo/pdo_dbh.c | 5 ----- ext/pdo/tests/pdo_quote_empty_string.phpt | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 ext/pdo/tests/pdo_quote_empty_string.phpt diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 3e24d125800ab..69ec51beb0f11 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -1147,11 +1147,6 @@ PHP_METHOD(PDO, quote) PDO_CONSTRUCT_CHECK; - if (str_len == 0) { - zend_argument_value_error(1, "cannot be empty"); - RETURN_THROWS(); - } - PDO_DBH_CLEAR_ERR(); if (!dbh->methods->quoter) { pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support quoting"); diff --git a/ext/pdo/tests/pdo_quote_empty_string.phpt b/ext/pdo/tests/pdo_quote_empty_string.phpt new file mode 100644 index 0000000000000..b3dce94ca5908 --- /dev/null +++ b/ext/pdo/tests/pdo_quote_empty_string.phpt @@ -0,0 +1,22 @@ +--TEST-- +PDO::quote() must accept empty string +--SKIPIF-- + +--FILE-- +quote('')); + +?> +--EXPECT-- +string(2) "''" From 86959ee19f5939935ce2d50a46f764e58a21bf31 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 11:17:27 +0100 Subject: [PATCH 43/50] Drop ValueError for empty lastInsertId() --- ext/pdo/pdo_dbh.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 69ec51beb0f11..936514fc12b58 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -949,11 +949,6 @@ PHP_METHOD(PDO, lastInsertId) PDO_CONSTRUCT_CHECK; - if (name && namelen == 0) { - zend_argument_value_error(1, "cannot be empty"); - RETURN_THROWS(); - } - PDO_DBH_CLEAR_ERR(); if (!dbh->methods->last_id) { From 8944242d05915193b7caa375ceb83d72d1bd0e83 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 11:23:58 +0100 Subject: [PATCH 44/50] Accept strings values again for integer attribute values --- ext/pdo/pdo_dbh.c | 3 ++- ext/pdo/tests/bug_44159.phpt | 2 +- ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt | 1 - ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt | 1 - 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 936514fc12b58..e07ffff49667c 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -678,8 +678,9 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* { zend_long lval; +/* 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_FALSE && Z_TYPE_P(value) != IS_TRUE) { \ + 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 int for selected attribute, %s given", zend_zval_type_name(value)); \ return FAILURE; \ } \ diff --git a/ext/pdo/tests/bug_44159.phpt b/ext/pdo/tests/bug_44159.phpt index af212ed555e2f..fb700f7b0e58e 100644 --- a/ext/pdo/tests/bug_44159.phpt +++ b/ext/pdo/tests/bug_44159.phpt @@ -44,4 +44,4 @@ 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 int for selected attribute, null given bool(true) -TypeError: Attribute value must be int for selected attribute, string given +bool(true) diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt index da30f62f27b10..4c509729d43f9 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt @@ -160,7 +160,6 @@ error_reporting=E_ALL --EXPECTF-- TypeError: Attribute value must be int for selected attribute, array given TypeError: Attribute value must be int for selected attribute, stdClass given -TypeError: Attribute value must be 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 diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt index 61ee29d4d98c8..192c6eb1f979e 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt @@ -90,7 +90,6 @@ MySQLPDOTest::skip(); --EXPECTF-- Attribute value must be int for selected attribute, array given Attribute value must be int for selected attribute, stdClass given -Attribute value must be int for selected attribute, string given array(1) { [0]=> array(6) { From a0eedd854636ecefa29433e80fe7d70d1174697b Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 11:36:56 +0100 Subject: [PATCH 45/50] Review for pdo_stmt.c Promote a couple of confirmed cases to unconditional error Fix error messages --- ext/pdo/pdo_stmt.c | 22 ++++++++++------------ ext/pdo/tests/pdo_038.phpt | 12 +++++++----- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 3e3961a8bf8c8..1f93699b95cb6 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -470,10 +470,8 @@ static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, int *typ } if (colno >= stmt->column_count) { - /* TODO Should this be a ValueError? */ - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Invalid column index"); - ZVAL_FALSE(dest); - + zend_value_error("Invalid column index"); + ZVAL_NULL(dest); return; } @@ -697,7 +695,7 @@ static int make_callable_ex(pdo_stmt_t *stmt, zval *callable, zend_fcall_info * zend_type_error("%s", is_callable_error); efree(is_callable_error); } else { - zend_type_error("user-supplied function must be a valid callback"); + zend_type_error("User-supplied function must be a valid callback"); } return false; } @@ -824,9 +822,9 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h zend_value_error("Column index must be greater than or equal to 0"); return false; } - /* TODO Always a ValueError? */ + if (colno >= stmt->column_count) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "Invalid column index"); + zend_value_error("Invalid column index"); return false; } @@ -1150,14 +1148,14 @@ static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode switch(mode) { case PDO_FETCH_FUNC: if (!fetch_all) { - zend_argument_value_error(mode_arg_num, "can only use PDO::FETCH_FUNC in PDOStatement::fetchAll()"); + zend_value_error("Can only use PDO::FETCH_FUNC in PDOStatement::fetchAll()"); return 0; } return 1; case PDO_FETCH_LAZY: if (fetch_all) { - zend_argument_value_error(mode_arg_num, "cannot use PDO::FETCH_LAZY in PDOStatement::fetchAll()"); + zend_value_error("Cannot be PDO::FETCH_LAZY in PDOStatement::fetchAll()"); return 0; } /* fall through */ @@ -1171,7 +1169,6 @@ static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode return 0; } if (mode >= PDO_FETCH__MAX) { - /* TODO Better error message? */ zend_argument_value_error(mode_arg_num, "must be a bitmask of PDO::FETCH_* constants"); return 0; } @@ -1228,8 +1225,9 @@ PHP_METHOD(PDOStatement, fetchObject) PHP_STMT_GET_OBJ; PDO_STMT_CLEAR_ERR(); - /* use pdo_stmt_verify_mode() to set fetch mode for this specific statement */ - ZEND_ASSERT(pdo_stmt_verify_mode(stmt, PDO_FETCH_CLASS, 0, false)); + /* Use pdo_stmt_verify_mode() to set fetch mode for this specific statement */ + /* This should NOT fail */ + pdo_stmt_verify_mode(stmt, PDO_FETCH_CLASS, 0, false); old_ce = stmt->fetch.cls.ce; ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args); diff --git a/ext/pdo/tests/pdo_038.phpt b/ext/pdo/tests/pdo_038.phpt index 5851a9fe6fd7d..a2887f35dbf89 100644 --- a/ext/pdo/tests/pdo_038.phpt +++ b/ext/pdo/tests/pdo_038.phpt @@ -38,11 +38,13 @@ try { echo $e->getMessage(), \PHP_EOL; } var_dump(fetchColumn($stmt, 0)); -var_dump(fetchColumn($stmt, 1)); +try { + var_dump(fetchColumn($stmt, 1)); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} ?> ---EXPECTF-- +--EXPECT-- Column index must be greater than or equal to 0 string(1) "1" - -Warning: PDOStatement::fetchColumn(): SQLSTATE[HY000]: General error: Invalid column index in %s -bool(false) +Invalid column index From 2fd259fd6197a783518d43a46ef5b1dfc6c43f75 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 12:29:49 +0100 Subject: [PATCH 46/50] Revert ZPP to use variadics --- ext/pdo/pdo_dbh.c | 11 +++-- ext/pdo/pdo_stmt.c | 85 +++++++++++++++++++++++------------- ext/pdo/php_pdo_int.h | 5 +-- ext/pdo/tests/bug_44173.phpt | 10 ++--- 4 files changed, 66 insertions(+), 45 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index e07ffff49667c..68b94f2202cb3 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -1050,13 +1050,13 @@ PHP_METHOD(PDO, query) size_t statement_len; zend_long fetch_mode; zend_bool fetch_mode_is_null = 1; - zval *arg1 = NULL; - HashTable *constructor_args = NULL; + zval *args = NULL; + uint32_t num_args = 0; pdo_dbh_object_t *dbh_obj = Z_PDO_OBJECT_P(ZEND_THIS); pdo_dbh_t *dbh = dbh_obj->inner; - if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "s|l!z!h!", &statement, &statement_len, - &fetch_mode, &fetch_mode_is_null, &arg1, &constructor_args)) { + if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS(), "s|l!*", &statement, &statement_len, + &fetch_mode, &fetch_mode_is_null, &args, &num_args)) { RETURN_THROWS(); } @@ -1092,8 +1092,7 @@ PHP_METHOD(PDO, query) if (dbh->methods->preparer(dbh, statement, statement_len, stmt, NULL)) { PDO_STMT_CLEAR_ERR(); - if (fetch_mode_is_null || pdo_stmt_setup_fetch_mode(stmt, ZEND_NUM_ARGS(), - fetch_mode, 2, arg1, 3, constructor_args, 4)) { + if (fetch_mode_is_null || pdo_stmt_setup_fetch_mode(stmt, fetch_mode, 2, args, num_args)) { /* now execute the statement */ PDO_STMT_CLEAR_ERR(); if (stmt->methods->executer(stmt)) { diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 1f93699b95cb6..5321c8352870a 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1736,11 +1736,22 @@ PHP_METHOD(PDOStatement, getColumnMeta) /* {{{ Changes the default fetch mode for subsequent fetches (params have different meaning for different fetch modes) */ -bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_long fetch_mode, - uint32_t mode_arg_num, zval *arg1, uint32_t arg1_arg_num, HashTable *constructor_args, - uint32_t constructor_arg_num) +bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_arg_num, + zval *args, uint32_t variadic_num_args) { int flags = 0; + uint32_t arg1_arg_num = mode_arg_num + 1; + uint32_t constructor_arg_num = mode_arg_num + 2; + uint32_t total_num_args = mode_arg_num + variadic_num_args; + zval arg1, constructor_args; + + /* Assign variadic params to dedicated ones */ + if (variadic_num_args >= 1) { + arg1 = args[0]; + if (variadic_num_args >= 2) { + constructor_args = args[1]; + } + } switch (stmt->default_fetch_type) { case PDO_FETCH_INTO: @@ -1755,13 +1766,13 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l stmt->default_fetch_type = PDO_FETCH_BOTH; - flags = fetch_mode & PDO_FETCH_FLAGS; + flags = mode & PDO_FETCH_FLAGS; - if (!pdo_stmt_verify_mode(stmt, fetch_mode, mode_arg_num, false)) { + if (!pdo_stmt_verify_mode(stmt, mode, mode_arg_num, false)) { return false; } - switch (fetch_mode & ~PDO_FETCH_FLAGS) { + switch (mode & ~PDO_FETCH_FLAGS) { case PDO_FETCH_USE_DEFAULT: case PDO_FETCH_LAZY: case PDO_FETCH_ASSOC: @@ -1773,7 +1784,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l case PDO_FETCH_KEY_PAIR: if (total_num_args != mode_arg_num) { zend_string *func = get_active_function_or_method_name(); - zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + zend_argument_count_error("%s() expects exactly %d arguments for the fetch mode provided, %d given", ZSTR_VAL(func), mode_arg_num, total_num_args); zend_string_release(func); return false; @@ -1783,20 +1794,21 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l case PDO_FETCH_COLUMN: if (total_num_args != arg1_arg_num) { zend_string *func = get_active_function_or_method_name(); - zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + zend_argument_count_error("%s() expects exactly %d arguments for the fetch mode provided, %d given", ZSTR_VAL(func), arg1_arg_num, total_num_args); zend_string_release(func); return false; } - if (Z_TYPE_P(arg1) != IS_LONG) { - zend_argument_type_error(arg1_arg_num, "must be int, %s given", zend_zval_type_name(arg1)); + /* arg1 is initialized at the start */ + if (Z_TYPE(arg1) != IS_LONG) { + zend_argument_type_error(arg1_arg_num, "must be int, %s given", zend_zval_type_name(&arg1)); return false; } - if (Z_LVAL_P(arg1) < 0) { + if (Z_LVAL(arg1) < 0) { zend_argument_value_error(arg1_arg_num, "must be greater than or equal to 0"); return false; } - stmt->fetch.column = Z_LVAL_P(arg1); + stmt->fetch.column = Z_LVAL(arg1); break; case PDO_FETCH_CLASS: @@ -1804,9 +1816,9 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); /* Gets its class name from 1st column */ if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) { - if (arg1 || constructor_args) { + if (variadic_num_args != 0) { zend_string *func = get_active_function_or_method_name(); - zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + zend_argument_count_error("%s() expects exactly %d arguments for the fetch mode provided, %d given", ZSTR_VAL(func), mode_arg_num, total_num_args); zend_string_release(func); return false; @@ -1814,26 +1826,36 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l stmt->fetch.cls.ce = NULL; } else { zend_class_entry *cep; - - if (!arg1 || total_num_args > constructor_arg_num) { + if (variadic_num_args == 0) { + zend_string *func = get_active_function_or_method_name(); + zend_argument_count_error("%s() expects at least %d arguments for the fetch mode provided, %d given", + ZSTR_VAL(func), arg1_arg_num, total_num_args); + zend_string_release(func); + return false; + } + /* constructor_arguments can be null/not passed */ + if (variadic_num_args > 2) { zend_string *func = get_active_function_or_method_name(); - zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + zend_argument_count_error("%s() expects at most %d arguments for the fetch mode provided, %d given", ZSTR_VAL(func), constructor_arg_num, total_num_args); zend_string_release(func); return false; } - if (Z_TYPE_P(arg1) != IS_STRING) { - zend_argument_type_error(arg1_arg_num, "must be string, %s given", zend_zval_type_name(arg1)); + /* arg1 is initialized at the start */ + if (Z_TYPE(arg1) != IS_STRING) { + zend_argument_type_error(arg1_arg_num, "must be string, %s given", zend_zval_type_name(&arg1)); return false; } - cep = zend_lookup_class(Z_STR_P(arg1)); + cep = zend_lookup_class(Z_STR(arg1)); if (!cep) { zend_argument_type_error(arg1_arg_num, "must be a valid class"); return false; } stmt->fetch.cls.ce = cep; - if (constructor_args && zend_hash_num_elements(constructor_args)) { - ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(constructor_args)); + + /* constructor_args is initialized at the start if variadic_num_args == 2 */ + if (variadic_num_args == 2 && zend_hash_num_elements(Z_ARRVAL(constructor_args))) { + ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(Z_ARRVAL(constructor_args))); } } @@ -1843,24 +1865,25 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l case PDO_FETCH_INTO: if (total_num_args != arg1_arg_num) { zend_string *func = get_active_function_or_method_name(); - zend_argument_count_error("%s() expects exactly %d argument for the fetch mode provided, %d given", + zend_argument_count_error("%s() expects exactly %d arguments for the fetch mode provided, %d given", ZSTR_VAL(func), arg1_arg_num, total_num_args); zend_string_release(func); return false; } - if (Z_TYPE_P(arg1) != IS_OBJECT) { - zend_argument_type_error(arg1_arg_num, "must be object, %s given", zend_zval_type_name(arg1)); + /* arg1 is initialized at the start */ + if (Z_TYPE(arg1) != IS_OBJECT) { + zend_argument_type_error(arg1_arg_num, "must be object, %s given", zend_zval_type_name(&arg1)); return false; } - ZVAL_COPY(&stmt->fetch.into, arg1); + ZVAL_COPY(&stmt->fetch.into, &arg1); break; default: zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants"); return false; } - stmt->default_fetch_type = fetch_mode; + stmt->default_fetch_type = mode; return true; } @@ -1868,11 +1891,11 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_l PHP_METHOD(PDOStatement, setFetchMode) { zend_long fetch_mode; - zval *arg1 = NULL; - HashTable *constructor_args = NULL; + zval *args = NULL; + uint32_t num_args = 0; /* Support null for constructor arguments for BC */ - if (zend_parse_parameters(ZEND_NUM_ARGS(), "l|z!h!", &fetch_mode, &arg1, &constructor_args) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "l*", &fetch_mode, &args, &num_args) == FAILURE) { RETURN_THROWS(); } @@ -1880,7 +1903,7 @@ PHP_METHOD(PDOStatement, setFetchMode) do_fetch_opt_finish(stmt, 1); - if (!pdo_stmt_setup_fetch_mode(stmt, ZEND_NUM_ARGS(), fetch_mode, 1, arg1, 2, constructor_args, 3)) { + if (!pdo_stmt_setup_fetch_mode(stmt, fetch_mode, 1, args, num_args)) { RETURN_THROWS(); } diff --git a/ext/pdo/php_pdo_int.h b/ext/pdo/php_pdo_int.h index 4f37d26358556..7f992a5fd0de2 100644 --- a/ext/pdo/php_pdo_int.h +++ b/ext/pdo/php_pdo_int.h @@ -40,9 +40,8 @@ void pdo_dbstmt_free_storage(zend_object *std); zend_object_iterator *pdo_stmt_iter_get(zend_class_entry *ce, zval *object, int by_ref); extern zend_object_handlers pdo_dbstmt_object_handlers; int pdo_stmt_describe_columns(pdo_stmt_t *stmt); -bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, uint32_t total_num_args, zend_long mode, - uint32_t mode_arg_num, zval *arg1, uint32_t arg1_arg_num, HashTable *constructor_args, - uint32_t constructor_arg_num); +bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_arg_num, + zval *args, uint32_t variadic_num_args); extern zend_object *pdo_row_new(zend_class_entry *ce); extern const zend_function_entry pdo_row_functions[]; diff --git a/ext/pdo/tests/bug_44173.phpt b/ext/pdo/tests/bug_44173.phpt index 16b3c1fbd8dc1..0baa7973e5a70 100644 --- a/ext/pdo/tests/bug_44173.phpt +++ b/ext/pdo/tests/bug_44173.phpt @@ -72,9 +72,9 @@ try { ?> --EXPECT-- -PDO::query() expects exactly 2 argument for the fetch mode provided, 4 given +PDO::query() expects exactly 2 arguments for the fetch mode provided, 4 given PDO::query(): Argument #2 ($fetch_mode) must be of type ?int, string given -PDO::query() expects at most 4 arguments, 5 given -PDO::query() expects exactly 3 argument for the fetch mode provided, 2 given -PDO::query() expects exactly 3 argument for the fetch mode provided, 2 given -PDO::query() expects exactly 4 argument for the fetch mode provided, 2 given +PDO::query() expects at most 4 arguments for the fetch mode provided, 5 given +PDO::query() expects exactly 3 arguments for the fetch mode provided, 2 given +PDO::query() expects exactly 3 arguments for the fetch mode provided, 2 given +PDO::query() expects at least 3 arguments for the fetch mode provided, 2 given From c0d2d8b90e3ae9c914fc743f5bd64fd1a4b565ee Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 12:43:04 +0100 Subject: [PATCH 47/50] Do not advertise support for a PDO feature if it's not implemented by the ODBC driver And ammend PDO::quote() test to take into account drivers which do not support the feature --- ext/pdo/tests/pdo_quote_empty_string.phpt | 19 ++++++++++++++----- ext/pdo_odbc/odbc_driver.c | 8 +++++--- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/ext/pdo/tests/pdo_quote_empty_string.phpt b/ext/pdo/tests/pdo_quote_empty_string.phpt index b3dce94ca5908..214d1014b9713 100644 --- a/ext/pdo/tests/pdo_quote_empty_string.phpt +++ b/ext/pdo/tests/pdo_quote_empty_string.phpt @@ -1,5 +1,5 @@ --TEST-- -PDO::quote() must accept empty string +PDO::quote() must accept empty string for drivers which support this feature --SKIPIF-- quote('')); +$pdo = PDOTest::factory(); +$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); +try { + $result = $pdo->quote(''); + if (!is_string($result)) { + var_dump($result); + } +} catch (\PDOException) { + // Do nothing as quoting is not supported with this driver +} ?> +DONE + --EXPECT-- -string(2) "''" +DONE diff --git a/ext/pdo_odbc/odbc_driver.c b/ext/pdo_odbc/odbc_driver.c index 5b35455ede1b3..5631da7a36e50 100644 --- a/ext/pdo_odbc/odbc_driver.c +++ b/ext/pdo_odbc/odbc_driver.c @@ -257,12 +257,14 @@ static zend_long odbc_handle_doer(pdo_dbh_t *dbh, const char *sql, size_t sql_le return row_count; } +/* TODO: Do ODBC quoter static int odbc_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unquotedlen, char **quoted, size_t *quotedlen, enum pdo_param_type param_type ) { - /* pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data; */ - /* TODO: figure it out */ + // pdo_odbc_db_handle *H = (pdo_odbc_db_handle *)dbh->driver_data; + // TODO: figure it out return 0; } +*/ static int odbc_handle_begin(pdo_dbh_t *dbh) { @@ -373,7 +375,7 @@ static const struct pdo_dbh_methods odbc_methods = { odbc_handle_closer, odbc_handle_preparer, odbc_handle_doer, - odbc_handle_quoter, + NULL, /* quoter */ odbc_handle_begin, odbc_handle_commit, odbc_handle_rollback, From e4a13d0ccc556f454d089aa03b356d844070ee34 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 14:43:24 +0100 Subject: [PATCH 48/50] Another round of review Indentation Error message Use variadic number position for pdo_stmt_setup_fetch_mode() param cehcks Use values from *args directly instead of preassigning them in pdo_stmt_setup_fetch_mode() Drop todo comment --- ext/pdo/pdo_stmt.c | 86 +++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 5321c8352870a..71a67c5c69ba2 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -37,7 +37,7 @@ #define PHP_STMT_GET_OBJ \ pdo_stmt_t *stmt = Z_PDO_STMT_P(ZEND_THIS); \ if (!stmt->dbh) { \ - zend_throw_error(NULL, "PDO Object is uninitialized"); \ + zend_throw_error(NULL, "PDO object is uninitialized"); \ RETURN_THROWS(); \ } \ @@ -260,7 +260,7 @@ static int really_register_bound_param(struct pdo_bound_param_data *param, pdo_s for (i = 0; i < stmt->column_count; i++) { if (ZSTR_LEN(stmt->columns[i].name) == ZSTR_LEN(param->name) && - strncmp(ZSTR_VAL(stmt->columns[i].name), ZSTR_VAL(param->name), ZSTR_LEN(param->name) + 1) == 0) { + strncmp(ZSTR_VAL(stmt->columns[i].name), ZSTR_VAL(param->name), ZSTR_LEN(param->name) + 1) == 0) { param->paramno = i; break; } @@ -404,9 +404,9 @@ PHP_METHOD(PDOStatement, execute) if (PDO_PLACEHOLDER_NONE == stmt->supports_placeholders) { /* handle the emulated parameter binding, - * stmt->active_query_string holds the query with binds expanded and + * stmt->active_query_string holds the query with binds expanded and * quoted. - */ + */ /* string is leftover from previous calls so PDOStatement::debugDumpParams() can access */ if (stmt->active_query_string && stmt->active_query_string != stmt->query_string) { @@ -730,7 +730,7 @@ static void do_fetch_opt_finish(pdo_stmt_t *stmt, int free_ctor_agrs) /* {{{ */ /* fci.size is used to check if it is valid */ if (stmt->fetch.cls.fci.size && stmt->fetch.cls.fci.params) { if (!Z_ISUNDEF(stmt->fetch.cls.ctor_args)) { - /* Added to free constructor arguments */ + /* Added to free constructor arguments */ zend_fcall_info_args_clear(&stmt->fetch.cls.fci, 1); } else { efree(stmt->fetch.cls.fci.params); @@ -1155,7 +1155,7 @@ static bool pdo_stmt_verify_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode case PDO_FETCH_LAZY: if (fetch_all) { - zend_value_error("Cannot be PDO::FETCH_LAZY in PDOStatement::fetchAll()"); + zend_argument_value_error(mode_arg_num, "cannot be PDO::FETCH_LAZY in PDOStatement::fetchAll()"); return 0; } /* fall through */ @@ -1194,7 +1194,7 @@ PHP_METHOD(PDOStatement, fetch) Z_PARAM_LONG(off) ZEND_PARSE_PARAMETERS_END(); - PHP_STMT_GET_OBJ; + PHP_STMT_GET_OBJ; PDO_STMT_CLEAR_ERR(); if (!pdo_stmt_verify_mode(stmt, how, 1, false)) { @@ -1578,7 +1578,7 @@ PHP_METHOD(PDOStatement, errorCode) PHP_METHOD(PDOStatement, errorInfo) { int error_count; - int error_count_diff = 0; + int error_count_diff = 0; int error_expected_count = 3; ZEND_PARSE_PARAMETERS_NONE(); @@ -1743,15 +1743,6 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a uint32_t arg1_arg_num = mode_arg_num + 1; uint32_t constructor_arg_num = mode_arg_num + 2; uint32_t total_num_args = mode_arg_num + variadic_num_args; - zval arg1, constructor_args; - - /* Assign variadic params to dedicated ones */ - if (variadic_num_args >= 1) { - arg1 = args[0]; - if (variadic_num_args >= 2) { - constructor_args = args[1]; - } - } switch (stmt->default_fetch_type) { case PDO_FETCH_INTO: @@ -1782,7 +1773,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a case PDO_FETCH_BOUND: case PDO_FETCH_NAMED: case PDO_FETCH_KEY_PAIR: - if (total_num_args != mode_arg_num) { + if (variadic_num_args != 0) { zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly %d arguments for the fetch mode provided, %d given", ZSTR_VAL(func), mode_arg_num, total_num_args); @@ -1792,26 +1783,26 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a break; case PDO_FETCH_COLUMN: - if (total_num_args != arg1_arg_num) { + if (variadic_num_args != 1) { zend_string *func = get_active_function_or_method_name(); zend_argument_count_error("%s() expects exactly %d arguments for the fetch mode provided, %d given", ZSTR_VAL(func), arg1_arg_num, total_num_args); zend_string_release(func); return false; } - /* arg1 is initialized at the start */ - if (Z_TYPE(arg1) != IS_LONG) { - zend_argument_type_error(arg1_arg_num, "must be int, %s given", zend_zval_type_name(&arg1)); + if (Z_TYPE(args[0]) != IS_LONG) { + zend_argument_type_error(arg1_arg_num, "must be int, %s given", zend_zval_type_name(&args[0])); return false; } - if (Z_LVAL(arg1) < 0) { + if (Z_LVAL(args[0]) < 0) { zend_argument_value_error(arg1_arg_num, "must be greater than or equal to 0"); return false; } - stmt->fetch.column = Z_LVAL(arg1); + stmt->fetch.column = Z_LVAL(args[0]); break; - case PDO_FETCH_CLASS: + case PDO_FETCH_CLASS: { + HashTable *constructor_args = NULL; /* Undef constructor arguments */ ZVAL_UNDEF(&stmt->fetch.cls.ctor_args); /* Gets its class name from 1st column */ @@ -1841,27 +1832,38 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a zend_string_release(func); return false; } - /* arg1 is initialized at the start */ - if (Z_TYPE(arg1) != IS_STRING) { - zend_argument_type_error(arg1_arg_num, "must be string, %s given", zend_zval_type_name(&arg1)); + if (Z_TYPE(args[0]) != IS_STRING) { + zend_argument_type_error(arg1_arg_num, "must be string, %s given", zend_zval_type_name(&args[0])); return false; } - cep = zend_lookup_class(Z_STR(arg1)); + cep = zend_lookup_class(Z_STR(args[0])); if (!cep) { zend_argument_type_error(arg1_arg_num, "must be a valid class"); return false; } + /* Verify constructor_args (args[1]) is ?array */ + /* TODO: Improve logic? */ + if (variadic_num_args == 2) { + if (Z_TYPE(args[1]) != IS_NULL && Z_TYPE(args[1]) != IS_ARRAY) { + zend_argument_type_error(constructor_arg_num, "must be ?array, %s given", + zend_zval_type_name(&args[1])); + return false; + } + if (Z_TYPE(args[1]) == IS_ARRAY && zend_hash_num_elements(Z_ARRVAL(args[1]))) { + constructor_args = Z_ARRVAL(args[1]); + } + } stmt->fetch.cls.ce = cep; - /* constructor_args is initialized at the start if variadic_num_args == 2 */ - if (variadic_num_args == 2 && zend_hash_num_elements(Z_ARRVAL(constructor_args))) { - ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(Z_ARRVAL(constructor_args))); + /* If constructor arguments are present and not empty */ + if (constructor_args) { + ZVAL_ARR(&stmt->fetch.cls.ctor_args, zend_array_dup(constructor_args)); } } do_fetch_class_prepare(stmt); break; - + } case PDO_FETCH_INTO: if (total_num_args != arg1_arg_num) { zend_string *func = get_active_function_or_method_name(); @@ -1870,13 +1872,12 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a zend_string_release(func); return false; } - /* arg1 is initialized at the start */ - if (Z_TYPE(arg1) != IS_OBJECT) { - zend_argument_type_error(arg1_arg_num, "must be object, %s given", zend_zval_type_name(&arg1)); + if (Z_TYPE(args[0]) != IS_OBJECT) { + zend_argument_type_error(arg1_arg_num, "must be object, %s given", zend_zval_type_name(&args[0])); return false; } - ZVAL_COPY(&stmt->fetch.into, &arg1); + ZVAL_COPY(&stmt->fetch.into, &args[0]); break; default: zend_argument_value_error(mode_arg_num, "must be one of the PDO::FETCH_* constants"); @@ -1892,7 +1893,7 @@ PHP_METHOD(PDOStatement, setFetchMode) { zend_long fetch_mode; zval *args = NULL; - uint32_t num_args = 0; + uint32_t num_args = 0; /* Support null for constructor arguments for BC */ if (zend_parse_parameters(ZEND_NUM_ARGS(), "l*", &fetch_mode, &args, &num_args) == FAILURE) { @@ -2010,7 +2011,6 @@ PHP_METHOD(PDOStatement, debugDumpParams) PHP_STMT_GET_OBJ; - /* TODO: Change to assertion? */ if (out == NULL) { RETURN_FALSE; } @@ -2340,7 +2340,7 @@ static zval *row_prop_read(zend_object *object, zend_string *name, int type, voi * numbers */ for (colno = 0; colno < stmt->column_count; colno++) { if (ZSTR_LEN(stmt->columns[colno].name) == ZSTR_LEN(name) && - strncmp(ZSTR_VAL(stmt->columns[colno].name), ZSTR_VAL(name), ZSTR_LEN(name)) == 0) { + strncmp(ZSTR_VAL(stmt->columns[colno].name), ZSTR_VAL(name), ZSTR_LEN(name)) == 0) { fetch_value(stmt, rv, colno, NULL); return rv; } @@ -2382,7 +2382,7 @@ static zval *row_dim_read(zend_object *object, zval *member, int type, zval *rv) * numbers */ for (colno = 0; colno < stmt->column_count; colno++) { if (ZSTR_LEN(stmt->columns[colno].name) == Z_STRLEN_P(member) && - strncmp(ZSTR_VAL(stmt->columns[colno].name), Z_STRVAL_P(member), Z_STRLEN_P(member)) == 0) { + strncmp(ZSTR_VAL(stmt->columns[colno].name), Z_STRVAL_P(member), Z_STRLEN_P(member)) == 0) { fetch_value(stmt, rv, colno, NULL); return rv; } @@ -2424,7 +2424,7 @@ static int row_prop_exists(zend_object *object, zend_string *name, int check_emp * numbers */ for (colno = 0; colno < stmt->column_count; colno++) { if (ZSTR_LEN(stmt->columns[colno].name) == ZSTR_LEN(name) && - strncmp(ZSTR_VAL(stmt->columns[colno].name), ZSTR_VAL(name), ZSTR_LEN(name)) == 0) { + strncmp(ZSTR_VAL(stmt->columns[colno].name), ZSTR_VAL(name), ZSTR_LEN(name)) == 0) { int res; zval val; @@ -2464,7 +2464,7 @@ static int row_dim_exists(zend_object *object, zval *member, int check_empty) * numbers */ for (colno = 0; colno < stmt->column_count; colno++) { if (ZSTR_LEN(stmt->columns[colno].name) == Z_STRLEN_P(member) && - strncmp(ZSTR_VAL(stmt->columns[colno].name), Z_STRVAL_P(member), Z_STRLEN_P(member)) == 0) { + strncmp(ZSTR_VAL(stmt->columns[colno].name), Z_STRVAL_P(member), Z_STRLEN_P(member)) == 0) { int res; zval val; From 080465f4e29b3dd86f66b0abb44a1291f660b4a3 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 15:40:17 +0100 Subject: [PATCH 49/50] Minor review Indent (again) Drop unnecessary cehck Improve title + SKIPIF for PDO SQLite test --- ext/pdo/pdo_dbh.c | 2 +- ext/pdo/pdo_stmt.c | 4 ---- ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt | 9 ++------- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 68b94f2202cb3..b8f65f51dcf3a 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -1051,7 +1051,7 @@ PHP_METHOD(PDO, query) zend_long fetch_mode; zend_bool fetch_mode_is_null = 1; zval *args = NULL; - uint32_t num_args = 0; + uint32_t num_args = 0; pdo_dbh_object_t *dbh_obj = Z_PDO_OBJECT_P(ZEND_THIS); pdo_dbh_t *dbh = dbh_obj->inner; diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 71a67c5c69ba2..4f3c198479394 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1225,10 +1225,6 @@ PHP_METHOD(PDOStatement, fetchObject) PHP_STMT_GET_OBJ; PDO_STMT_CLEAR_ERR(); - /* Use pdo_stmt_verify_mode() to set fetch mode for this specific statement */ - /* This should NOT fail */ - pdo_stmt_verify_mode(stmt, PDO_FETCH_CLASS, 0, false); - old_ce = stmt->fetch.cls.ce; ZVAL_COPY_VALUE(&old_ctor_args, &stmt->fetch.cls.ctor_args); old_arg_count = stmt->fetch.cls.fci.param_count; diff --git a/ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt b/ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt index ced86346d5c54..fc30f1d21cc23 100644 --- a/ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt +++ b/ext/pdo_sqlite/tests/bug_44159_sqlite_version.phpt @@ -1,13 +1,8 @@ --TEST-- -PDO Common: Bug #44159 (Crash: $pdo->setAttribute(PDO::STATEMENT_ATTR_CLASS, NULL)): SQLite variant +PDO Common: Bug #44159: SQLite variant --SKIPIF-- --FILE-- Date: Mon, 28 Sep 2020 16:15:47 +0100 Subject: [PATCH 50/50] Fix type error messages --- ext/pdo/pdo_dbh.c | 6 +++--- ext/pdo/pdo_stmt.c | 14 +++++++------- ext/pdo/tests/bug_44159.phpt | 2 +- ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt | 4 ++-- .../tests/pdo_mysql_attr_oracle_nulls.phpt | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index b8f65f51dcf3a..0f5eaab2e6e62 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -531,7 +531,7 @@ PHP_METHOD(PDO, prepare) } 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 ctor_args must be ?array, %s given", + zend_type_error("PDO::ATTR_STATEMENT_CLASS ctor_args must be of type ?array, %s given", zend_zval_type_name(value)); RETURN_THROWS(); } @@ -681,7 +681,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* /* 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 int for selected attribute, %s given", zend_zval_type_name(value)); \ + zend_type_error("Attribute value must be of type int for selected attribute, %s given", zend_zval_type_name(value)); \ return FAILURE; \ } \ @@ -787,7 +787,7 @@ static int 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 ctor_args must be ?array, %s given", + zend_type_error("PDO::ATTR_STATEMENT_CLASS ctor_args must be of type ?array, %s given", zend_zval_type_name(value)); return FAILURE; } diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 4f3c198479394..0cec5d9955c53 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -1314,7 +1314,7 @@ PHP_METHOD(PDOStatement, fetchAll) /* Figure out correct class */ if (arg2) { if (Z_TYPE_P(arg2) != IS_STRING) { - zend_argument_type_error(2, "must be string, %s given", zend_zval_type_name(arg2)); + zend_argument_type_error(2, "must be of type string, %s given", zend_zval_type_name(arg2)); RETURN_THROWS(); } stmt->fetch.cls.ce = zend_fetch_class(Z_STR_P(arg2), ZEND_FETCH_CLASS_AUTO); @@ -1344,6 +1344,7 @@ PHP_METHOD(PDOStatement, fetchAll) RETURN_THROWS(); } if (arg2 == NULL) { + /* TODO use "must be of type callable" format? */ zend_argument_type_error(2, "must be a callable, null given"); RETURN_THROWS(); } @@ -1366,7 +1367,7 @@ PHP_METHOD(PDOStatement, fetchAll) if (arg2) { // Reuse convert_to_long(arg2); ? if (Z_TYPE_P(arg2) != IS_LONG) { - zend_argument_type_error(2, "must be int, %s given", zend_zval_type_name(arg2)); + zend_argument_type_error(2, "must be of type int, %s given", zend_zval_type_name(arg2)); RETURN_THROWS(); } if (Z_LVAL_P(arg2) < 0) { @@ -1787,7 +1788,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a return false; } if (Z_TYPE(args[0]) != IS_LONG) { - zend_argument_type_error(arg1_arg_num, "must be int, %s given", zend_zval_type_name(&args[0])); + zend_argument_type_error(arg1_arg_num, "must be of type int, %s given", zend_zval_type_name(&args[0])); return false; } if (Z_LVAL(args[0]) < 0) { @@ -1829,7 +1830,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a return false; } if (Z_TYPE(args[0]) != IS_STRING) { - zend_argument_type_error(arg1_arg_num, "must be string, %s given", zend_zval_type_name(&args[0])); + zend_argument_type_error(arg1_arg_num, "must be of type string, %s given", zend_zval_type_name(&args[0])); return false; } cep = zend_lookup_class(Z_STR(args[0])); @@ -1841,7 +1842,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a /* TODO: Improve logic? */ if (variadic_num_args == 2) { if (Z_TYPE(args[1]) != IS_NULL && Z_TYPE(args[1]) != IS_ARRAY) { - zend_argument_type_error(constructor_arg_num, "must be ?array, %s given", + zend_argument_type_error(constructor_arg_num, "must be of type ?array, %s given", zend_zval_type_name(&args[1])); return false; } @@ -1869,7 +1870,7 @@ bool pdo_stmt_setup_fetch_mode(pdo_stmt_t *stmt, zend_long mode, uint32_t mode_a return false; } if (Z_TYPE(args[0]) != IS_OBJECT) { - zend_argument_type_error(arg1_arg_num, "must be object, %s given", zend_zval_type_name(&args[0])); + zend_argument_type_error(arg1_arg_num, "must be of type object, %s given", zend_zval_type_name(&args[0])); return false; } @@ -1891,7 +1892,6 @@ PHP_METHOD(PDOStatement, setFetchMode) zval *args = NULL; uint32_t num_args = 0; - /* Support null for constructor arguments for BC */ if (zend_parse_parameters(ZEND_NUM_ARGS(), "l*", &fetch_mode, &args, &num_args) == FAILURE) { RETURN_THROWS(); } diff --git a/ext/pdo/tests/bug_44159.phpt b/ext/pdo/tests/bug_44159.phpt index fb700f7b0e58e..b5601eed41c47 100644 --- a/ext/pdo/tests/bug_44159.phpt +++ b/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 int for selected attribute, null given +TypeError: Attribute value must be of type int for selected attribute, null given bool(true) bool(true) diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt index 4c509729d43f9..d6ebd87f60327 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt @@ -158,8 +158,8 @@ error_reporting=E_ALL print "done!\n"; ?> --EXPECTF-- -TypeError: Attribute value must be int for selected attribute, array given -TypeError: Attribute value must be int for selected attribute, stdClass given +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 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 diff --git a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt index 192c6eb1f979e..6d922a037def8 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_oracle_nulls.phpt @@ -88,8 +88,8 @@ MySQLPDOTest::skip(); print "done!"; ?> --EXPECTF-- -Attribute value must be int for selected attribute, array given -Attribute value must be int for selected attribute, stdClass given +Attribute value must be of type int for selected attribute, array given +Attribute value must be of type int for selected attribute, stdClass given array(1) { [0]=> array(6) {