Skip to content

PDO Unconditional errors #6212

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

Closed
wants to merge 50 commits into from
Closed

PDO Unconditional errors #6212

Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
2435c87
Fix a logical bug in PDO
Girgias Sep 24, 2020
e280e5e
Column index must be >= 0 (just 1 case) and todo comment.
Girgias Sep 24, 2020
96770fa
Refactor PDO setFetchMode
Girgias Sep 25, 2020
c5a4c3e
Drop ZPP check in PDO Mysqli test...
Girgias Sep 25, 2020
4c75cae
Make certain error conditions always error in PDO::fetchAll()
Girgias Sep 25, 2020
2cf849b
Unconditonal error for values less than a minimum
Girgias Sep 25, 2020
220d89f
Add todo comments
Girgias Sep 25, 2020
b2bd8b6
ValueError for negative column index
Girgias Sep 25, 2020
156ec13
Assertion + always throw on invalid Fetch mode
Girgias Sep 25, 2020
7bcf852
ValueError for incorrect Fetch Flag usage
Girgias Sep 25, 2020
d92274a
Make invalid user callable throw a type error
Girgias Sep 25, 2020
4028d9f
More TODOs
Girgias Sep 25, 2020
1615a93
Todo everything...
Girgias Sep 25, 2020
2f9bb0e
ValueError for empty query in PDO::exec()
Girgias Sep 25, 2020
ad8a780
Preliminary Type/ValueError for setting PDO attributes
Girgias Sep 25, 2020
ba6ce47
Attempt to fix SQLite test
Girgias Sep 25, 2020
dd7c891
TypeErrors for invalid attribute options
Girgias Sep 25, 2020
d1d6393
Type Error for invalid options in PDO::prepare()
Girgias Sep 25, 2020
e711702
Drop superflous PDO_STMT_CLEAR_ERR();
Girgias Sep 25, 2020
b2a0a2e
Fix NULL pointer access
Girgias Sep 25, 2020
d5d51e8
Split bug 44159 SQLite difference in behaviour to a dedicated variant
Girgias Sep 26, 2020
fb87fcd
Fix memory leaks
Girgias Sep 26, 2020
d95763d
Make unintialized PDO object an Error
Girgias Sep 26, 2020
dc3af24
Drop string type as acceptable type when checking for integer
Girgias Sep 26, 2020
df49654
Uncodnitional error for attempting to write/delete read-only property
Girgias Sep 26, 2020
8c1f0ff
Tiny Refactor
Girgias Sep 26, 2020
2f2f5ba
Make a check an assertion
Girgias Sep 26, 2020
2b57e46
Make unitialized object an error
Girgias Sep 26, 2020
095513b
Refactor PDOStatement::fetchAll()
Girgias Sep 26, 2020
d56559f
Fix stubs
Girgias Sep 26, 2020
66f35b5
Refactor PDOStatement::setAttribute() to be more explicit
Girgias Sep 26, 2020
7716b91
Add a TODO comment
Girgias Sep 26, 2020
8e9fa15
Redfine todo comment
Girgias Sep 26, 2020
1725e31
Introduce ValueError for empty bind param names
Girgias Sep 26, 2020
79e9444
Fix stubs after making uninitialized object return Error
Girgias Sep 26, 2020
64349d3
Introduce ValueError for empty string arguments
Girgias Sep 26, 2020
a52ce29
Revert ValueError for empty username
Girgias Sep 26, 2020
d94521b
Make fetch argument variadic
Girgias Sep 27, 2020
ee37a1d
Fix test after making fetch args variadic
Girgias Sep 28, 2020
89b233c
Review for pdo_dbh.c
Girgias Sep 28, 2020
978b43d
Drop todo comments in pdo_sql_parser.re
Girgias Sep 28, 2020
909651e
Revert + add test for empty string in PDO::qutoe()
Girgias Sep 28, 2020
86959ee
Drop ValueError for empty lastInsertId()
Girgias Sep 28, 2020
8944242
Accept strings values again for integer attribute values
Girgias Sep 28, 2020
a0eedd8
Review for pdo_stmt.c
Girgias Sep 28, 2020
2fd259f
Revert ZPP to use variadics
Girgias Sep 28, 2020
c0d2d8b
Do not advertise support for a PDO feature if it's not implemented by…
Girgias Sep 28, 2020
e4a13d0
Another round of review
Girgias Sep 28, 2020
080465f
Minor review
Girgias Sep 28, 2020
c949e24
Fix type error messages
Girgias Sep 28, 2020
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
150 changes: 76 additions & 74 deletions ext/pdo/pdo_dbh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,12 +425,10 @@ 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) {
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) {
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;
}
}
Expand Down Expand Up @@ -487,7 +485,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;
Expand All @@ -498,42 +496,44 @@ PHP_METHOD(PDO, prepare)
Z_PARAM_ARRAY(options)
ZEND_PARSE_PARAMETERS_END();

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
) {
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 (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 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 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 class must be a valid class");
RETURN_THROWS();
}
dbstmt_ce = pce;
if (!instanceof_function(dbstmt_ce, pdo_dbstmt_ce)) {
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 class must be derived from PDOStatement");
RETURN_THROWS();
}
if (dbstmt_ce->constructor && !(dbstmt_ce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) {
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(opt), 1)) != NULL) {
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 1)) != NULL) {
if (Z_TYPE_P(item) != IS_ARRAY) {
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 ctor_args must be ?array, %s given",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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();
}
ZVAL_COPY_VALUE(&ctor_args, item);
} else {
Expand All @@ -544,11 +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))) {
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;
Expand Down Expand Up @@ -679,10 +678,10 @@ 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_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)); \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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; \
} \

Expand All @@ -697,8 +696,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
dbh->error_mode = lval;
return SUCCESS;
default:
pdo_raise_impl_error(dbh, NULL, "HY000", "invalid error mode");
PDO_HANDLE_DBH_ERR();
zend_value_error("Error mode must be one of the PDO::ERRMODE_* constants");
return FAILURE;
}
return FAILURE;
Expand All @@ -713,8 +711,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
dbh->desired_case = lval;
return SUCCESS;
default:
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 the PDO::CASE_* constants");
return FAILURE;
}
return FAILURE;
Expand All @@ -729,7 +726,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) {
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 the default fetch mode");
return FAILURE;
}
}
Expand All @@ -738,7 +735,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) {
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;
Expand All @@ -761,28 +758,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
) {
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 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 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 class must be a valid class");
return FAILURE;
}
if (!instanceof_function(pce, pdo_dbstmt_ce)) {
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 class must be derived from PDOStatement");
return FAILURE;
}
if (pce->constructor && !(pce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) {
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;
Expand All @@ -792,11 +787,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) {
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 ctor_args must be ?array, %s given",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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;
}
ZVAL_COPY(&dbh->def_stmt_ctor_args, item);
Expand Down Expand Up @@ -927,10 +919,11 @@ PHP_METHOD(PDO, exec)
Z_PARAM_STRING(statement, statement_len)
ZEND_PARSE_PARAMETERS_END();

if (!statement_len) {
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);
Expand All @@ -955,8 +948,10 @@ PHP_METHOD(PDO, lastInsertId)
Z_PARAM_STRING_OR_NULL(name, namelen)
ZEND_PARSE_PARAMETERS_END();

PDO_DBH_CLEAR_ERR();
PDO_CONSTRUCT_CHECK;

PDO_DBH_CLEAR_ERR();

if (!dbh->methods->last_id) {
pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support lastInsertId()");
RETURN_FALSE;
Expand Down Expand Up @@ -1060,13 +1055,20 @@ PHP_METHOD(PDO, query)
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!*", &statement, &statement_len,
&fetch_mode, &fetch_mode_is_null, &args, &num_args)) {
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");
Expand All @@ -1090,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 || SUCCESS == pdo_stmt_setup_fetch_mode(stmt, fetch_mode, args, num_args)) {

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)) {
Expand Down Expand Up @@ -1139,8 +1140,9 @@ PHP_METHOD(PDO, quote)
Z_PARAM_LONG(paramtype)
ZEND_PARSE_PARAMETERS_END();

PDO_DBH_CLEAR_ERR();
PDO_CONSTRUCT_CHECK;

PDO_DBH_CLEAR_ERR();
if (!dbh->methods->quoter) {
pdo_raise_impl_error(dbh, NULL, "IM001", "driver does not support quoting");
RETURN_FALSE;
Expand Down
Loading