From 7553c696c306d48499c71e4f165b0c93699c7b4a Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 19:35:31 +0100 Subject: [PATCH] Another pass making some failure states unconditional erros in PDO Also make internal function return type more accurate to inform usage --- ext/pdo/pdo_dbh.c | 21 ++++++-------- ext/pdo/pdo_stmt.c | 29 +++++++++---------- .../tests/pdo_mysql_attr_statement_class.phpt | 19 +++++------- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c index 0f5eaab2e6e62..2e81db2bbf52d 100644 --- a/ext/pdo/pdo_dbh.c +++ b/ext/pdo/pdo_dbh.c @@ -34,7 +34,7 @@ #include "zend_interfaces.h" #include "pdo_dbh_arginfo.h" -static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value); +static zend_result pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value); void pdo_throw_exception(unsigned int driver_errcode, char *driver_errmsg, pdo_error_type *pdo_error) { @@ -405,6 +405,7 @@ PHP_METHOD(PDO, __construct) continue; } ZVAL_DEREF(attr_value); + /* TODO: Check that this doesn't fail? */ pdo_dbh_attribute_set(dbh, long_key, attr_value); } ZEND_HASH_FOREACH_END(); } @@ -434,6 +435,9 @@ static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry } if (UNEXPECTED(object_init_ex(object, dbstmt_ce) != SUCCESS)) { + if (EXPECTED(!EG(exception))) { + zend_throw_error(NULL, "Cannot instantiate user-supplied statement class"); + } return NULL; } @@ -544,13 +548,8 @@ 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))) { - zend_throw_error(NULL, "Cannot instantiate user-supplied statement class"); - } - PDO_HANDLE_DBH_ERR(); - RETURN_FALSE; + RETURN_THROWS(); } stmt = Z_PDO_STMT_P(return_value); @@ -674,7 +673,7 @@ PHP_METHOD(PDO, inTransaction) } /* }}} */ -static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* {{{ */ +static zend_result pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* {{{ */ { zend_long lval; @@ -752,6 +751,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /* zval *item; if (dbh->is_persistent) { + /* TODO: ValueError/ PDOException? */ pdo_raise_impl_error(dbh, NULL, "HY000", "PDO::ATTR_STATEMENT_CLASS cannot be used with persistent PDO instances" ); @@ -1070,10 +1070,7 @@ PHP_METHOD(PDO, query) 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"); - } - return; + RETURN_THROWS(); } stmt = Z_PDO_STMT_P(return_value); diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 0cec5d9955c53..3222a617f07e9 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -41,7 +41,7 @@ RETURN_THROWS(); \ } \ -static inline int rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_param_data *param) /* {{{ */ +static inline bool rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_param_data *param) /* {{{ */ { if (stmt->bound_param_map) { /* rewriting :name to ? style. @@ -90,9 +90,9 @@ static inline int rewrite_name_to_position(pdo_stmt_t *stmt, struct pdo_bound_pa /* }}} */ /* trigger callback hook for parameters */ -static int dispatch_param_event(pdo_stmt_t *stmt, enum pdo_param_event event_type) /* {{{ */ +static bool dispatch_param_event(pdo_stmt_t *stmt, enum pdo_param_event event_type) /* {{{ */ { - int ret = 1, is_param = 1; + bool ret = 1, is_param = 1; struct pdo_bound_param_data *param; HashTable *ht; @@ -212,7 +212,7 @@ static void param_dtor(zval *el) /* {{{ */ } /* }}} */ -static int really_register_bound_param(struct pdo_bound_param_data *param, pdo_stmt_t *stmt, int is_param) /* {{{ */ +static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_stmt_t *stmt, bool is_param) /* {{{ */ { HashTable *hash; zval *parameter; @@ -381,12 +381,7 @@ PHP_METHOD(PDOStatement, execute) param.paramno = -1; } else { /* we're okay to be zero based here */ - /* num_index is unsignend - if (num_index < 0) { - pdo_raise_impl_error(stmt->dbh, stmt, "HY093", NULL); - RETURN_FALSE; - } - */ + /* num_index is unsignend */ param.paramno = num_index; } @@ -601,7 +596,7 @@ static inline void fetch_value(pdo_stmt_t *stmt, zval *dest, int colno, int *typ } /* }}} */ -static int do_fetch_common(pdo_stmt_t *stmt, enum pdo_fetch_orientation ori, zend_long offset) /* {{{ */ +static bool do_fetch_common(pdo_stmt_t *stmt, enum pdo_fetch_orientation ori, zend_long offset) /* {{{ */ { if (!stmt->executed) { return 0; @@ -652,7 +647,7 @@ static int do_fetch_common(pdo_stmt_t *stmt, enum pdo_fetch_orientation ori, zen } /* }}} */ -static int do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */ +static bool do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */ { zend_class_entry *ce = stmt->fetch.cls.ce; zend_fcall_info *fci = &stmt->fetch.cls.fci; @@ -677,8 +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)) { - /* 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"); + zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments"); return 0; } else { return 1; /* no ctor no args is also ok */ @@ -686,7 +680,7 @@ static int do_fetch_class_prepare(pdo_stmt_t *stmt) /* {{{ */ } /* }}} */ -static int make_callable_ex(pdo_stmt_t *stmt, zval *callable, zend_fcall_info * fci, zend_fcall_info_cache * fcc, int num_args) /* {{{ */ +static bool make_callable_ex(pdo_stmt_t *stmt, zval *callable, zend_fcall_info * fci, zend_fcall_info_cache * fcc, int num_args) /* {{{ */ { char *is_callable_error = NULL; @@ -809,6 +803,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h case PDO_FETCH_KEY_PAIR: if (stmt->column_count != 2) { + /* TODO: Error? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "PDO::FETCH_KEY_PAIR fetch mode requires the result set to contain exactly 2 columns."); return 0; } @@ -904,6 +899,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h break; case PDO_FETCH_INTO: + /* TODO: Make this an assertion and ensure this is true higher up? */ if (Z_ISUNDEF(stmt->fetch.into)) { /* TODO ArgumentCountError? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch-into object specified."); @@ -919,6 +915,7 @@ static bool do_fetch(pdo_stmt_t *stmt, zval *return_value, enum pdo_fetch_type h break; case PDO_FETCH_FUNC: + /* TODO: Make this an assertion and ensure this is true higher up? */ if (Z_ISUNDEF(stmt->fetch.func.function)) { /* TODO ArgumentCountError? */ pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "No fetch function specified"); @@ -1633,7 +1630,7 @@ PHP_METHOD(PDOStatement, setAttribute) /* {{{ Get an attribute */ -static int generic_stmt_attr_get(pdo_stmt_t *stmt, zval *return_value, zend_long attr) +static bool generic_stmt_attr_get(pdo_stmt_t *stmt, zval *return_value, zend_long attr) { switch (attr) { case PDO_ATTR_EMULATE_PREPARES: 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 6d536f9deddd2..77f231d28fa65 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt @@ -114,13 +114,15 @@ $db = MySQLPDOTest::factory(); // Yes, this is a fatal error and I want it to fail. abstract class mystatement6 extends mystatement5 { } - if (true !== ($tmp = $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, array('mystatement6', array('mystatement6'))))) - printf("[011] Expecting boolean/true got %s\n", var_export($tmp, true)); - $stmt = $db->query('SELECT id, label FROM test ORDER BY id ASC LIMIT 1'); + try { + $db->setAttribute(PDO::ATTR_STATEMENT_CLASS, ['mystatement6', ['mystatement6']]); + $stmt = $db->query('SELECT id, label FROM test ORDER BY id ASC LIMIT 1'); + } catch (\Error $e) { + echo get_class($e), ': ', $e->getMessage(), \PHP_EOL; + } - print "done!"; ?> ---EXPECTF-- +--EXPECT-- array(1) { [0]=> string(12) "PDOStatement" @@ -157,9 +159,4 @@ array(1) { string(1) "a" } } - -Fatal error: Uncaught Error: Cannot instantiate abstract class mystatement6 in %s:%d -Stack trace: -#0 %s(%d): PDO->query('SELECT id, labe...') -#1 {main} - thrown in %s on line %d +Error: Cannot instantiate abstract class mystatement6