Skip to content

Another pass making some failure states unconditional erros in PDO #6227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 28, 2020
Merged

Another pass making some failure states unconditional erros in PDO #6227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions ext/pdo/pdo_dbh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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"
);
Expand Down Expand Up @@ -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);

Expand Down
29 changes: 13 additions & 16 deletions ext/pdo/pdo_stmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -677,16 +672,15 @@ 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 */
}
}
/* }}} */

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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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.");
Expand All @@ -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");
Expand Down Expand Up @@ -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:
Expand Down
19 changes: 8 additions & 11 deletions ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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