Skip to content

Commit d3465b9

Browse files
GirgiasGirgias
committed
Review for pdo_dbh.c
Promote a couple of confirmed cases to unconditional error Fix error messages
1 parent e6a445f commit d3465b9

File tree

4 files changed

+30
-42
lines changed

4 files changed

+30
-42
lines changed

‎ext/pdo/pdo_dbh.c

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -428,8 +428,7 @@ static zval *pdo_stmt_instantiate(pdo_dbh_t *dbh, zval *object, zend_class_entry
428428
/* This implies an error within PDO if this does not hold */
429429
ZEND_ASSERT(Z_TYPE_P(ctor_args) == IS_ARRAY);
430430
if (!dbstmt_ce->constructor) {
431-
/* TODO Error? */
432-
pdo_raise_impl_error(dbh, NULL, "HY000", "user-supplied statement does not accept constructor arguments");
431+
zend_throw_error(NULL, "User-supplied statement does not accept constructor arguments");
433432
return NULL;
434433
}
435434
}
@@ -508,34 +507,31 @@ PHP_METHOD(PDO, prepare)
508507

509508
if (options && (value = zend_hash_index_find(Z_ARRVAL_P(options), PDO_ATTR_STATEMENT_CLASS)) != NULL) {
510509
if (Z_TYPE_P(value) != IS_ARRAY) {
511-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's value must be of type array, %s given",
510+
zend_type_error("PDO::ATTR_STATEMENT_CLASS value must be of type array, %s given",
512511
zend_zval_type_name(value));
513512
RETURN_THROWS();
514513
}
515514
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL) {
516-
zend_value_error("PDO::ATTR_STATEMENT_CLASS's value must be an array with the following "
517-
"format array(classname, array(ctor_args))");
515+
zend_value_error("PDO::ATTR_STATEMENT_CLASS value must be an array with the format "
516+
"array(classname, array(ctor_args))");
518517
RETURN_THROWS();
519518
}
520519
if (Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL) {
521-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be a valid class");
520+
zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be a valid class");
522521
RETURN_THROWS();
523522
}
524523
dbstmt_ce = pce;
525524
if (!instanceof_function(dbstmt_ce, pdo_dbstmt_ce)) {
526-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement");
525+
zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement");
527526
RETURN_THROWS();
528527
}
529528
if (dbstmt_ce->constructor && !(dbstmt_ce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) {
530-
/* TODO Always Error? */
531-
pdo_raise_impl_error(dbh, NULL, "HY000",
532-
"user-supplied statement class cannot have a public constructor");
533-
PDO_HANDLE_DBH_ERR();
534-
RETURN_FALSE;
529+
zend_type_error("User-supplied statement class cannot have a public constructor");
530+
RETURN_THROWS();
535531
}
536532
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 1)) != NULL) {
537533
if (Z_TYPE_P(item) != IS_ARRAY) {
538-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's ctor_args must be ?array, %s given",
534+
zend_type_error("PDO::ATTR_STATEMENT_CLASS ctor_args must be ?array, %s given",
539535
zend_zval_type_name(value));
540536
RETURN_THROWS();
541537
}
@@ -548,12 +544,10 @@ PHP_METHOD(PDO, prepare)
548544
ZVAL_COPY_VALUE(&ctor_args, &dbh->def_stmt_ctor_args);
549545
}
550546

547+
/* Need to check if pdo_stmt_instantiate() throws an exception unconditionally to see if can change the RETURN_FALSE; */
551548
if (!pdo_stmt_instantiate(dbh, return_value, dbstmt_ce, &ctor_args)) {
552549
if (EXPECTED(!EG(exception))) {
553-
/* TODO Error? */
554-
pdo_raise_impl_error(dbh, NULL, "HY000",
555-
"failed to instantiate user-supplied statement class"
556-
);
550+
zend_throw_error(NULL, "Cannot instantiate user-supplied statement class");
557551
}
558552
PDO_HANDLE_DBH_ERR();
559553
RETURN_FALSE;
@@ -701,7 +695,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
701695
dbh->error_mode = lval;
702696
return SUCCESS;
703697
default:
704-
zend_value_error("Error mode must be one of PDO::ERRMODE_* constants");
698+
zend_value_error("Error mode must be one of the PDO::ERRMODE_* constants");
705699
return FAILURE;
706700
}
707701
return FAILURE;
@@ -716,7 +710,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
716710
dbh->desired_case = lval;
717711
return SUCCESS;
718712
default:
719-
zend_value_error("Case folding mode must be one of PDO::CASE_* constants");
713+
zend_value_error("Case folding mode must be one of the PDO::CASE_* constants");
720714
return FAILURE;
721715
}
722716
return FAILURE;
@@ -731,7 +725,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
731725
zval *tmp;
732726
if ((tmp = zend_hash_index_find(Z_ARRVAL_P(value), 0)) != NULL && Z_TYPE_P(tmp) == IS_LONG) {
733727
if (Z_LVAL_P(tmp) == PDO_FETCH_INTO || Z_LVAL_P(tmp) == PDO_FETCH_CLASS) {
734-
zend_value_error("PDO::FETCH_INTO and PDO::FETCH_CLASS cannot be set as default fetch mode");
728+
zend_value_error("PDO::FETCH_INTO and PDO::FETCH_CLASS cannot be set as the default fetch mode");
735729
return FAILURE;
736730
}
737731
}
@@ -764,28 +758,25 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
764758
return FAILURE;
765759
}
766760
if (Z_TYPE_P(value) != IS_ARRAY) {
767-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's value must be of type array, %s given",
761+
zend_type_error("PDO::ATTR_STATEMENT_CLASS value must be of type array, %s given",
768762
zend_zval_type_name(value));
769763
return FAILURE;
770764
}
771765
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 0)) == NULL) {
772-
zend_value_error("PDO::ATTR_STATEMENT_CLASS's value must be an array with the following "
773-
"format array(classname, array(ctor_args))");
766+
zend_value_error("PDO::ATTR_STATEMENT_CLASS value must be an array with the format "
767+
"array(classname, array(ctor_args))");
774768
return FAILURE;
775769
}
776770
if (Z_TYPE_P(item) != IS_STRING || (pce = zend_lookup_class(Z_STR_P(item))) == NULL) {
777-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be a valid class");
771+
zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be a valid class");
778772
return FAILURE;
779773
}
780774
if (!instanceof_function(pce, pdo_dbstmt_ce)) {
781-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement");
775+
zend_type_error("PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement");
782776
return FAILURE;
783777
}
784778
if (pce->constructor && !(pce->constructor->common.fn_flags & (ZEND_ACC_PRIVATE|ZEND_ACC_PROTECTED))) {
785-
/* TODO Always Error? */
786-
pdo_raise_impl_error(dbh, NULL, "HY000",
787-
"user-supplied statement class cannot have a public constructor");
788-
PDO_HANDLE_DBH_ERR();
779+
zend_type_error("User-supplied statement class cannot have a public constructor");
789780
return FAILURE;
790781
}
791782
dbh->def_stmt_ce = pce;
@@ -795,7 +786,7 @@ static int pdo_dbh_attribute_set(pdo_dbh_t *dbh, zend_long attr, zval *value) /*
795786
}
796787
if ((item = zend_hash_index_find(Z_ARRVAL_P(value), 1)) != NULL) {
797788
if (Z_TYPE_P(item) != IS_ARRAY) {
798-
zend_type_error("PDO::ATTR_STATEMENT_CLASS's ctor_args must be ?array, %s given",
789+
zend_type_error("PDO::ATTR_STATEMENT_CLASS ctor_args must be ?array, %s given",
799790
zend_zval_type_name(value));
800791
return FAILURE;
801792
}

‎ext/pdo/tests/bug_44159.phpt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ foreach ($attrs as $attr) {
3939

4040
?>
4141
--EXPECT--
42-
TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, null given
43-
TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, int given
44-
TypeError: PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given
42+
TypeError: PDO::ATTR_STATEMENT_CLASS value must be of type array, null given
43+
TypeError: PDO::ATTR_STATEMENT_CLASS value must be of type array, int given
44+
TypeError: PDO::ATTR_STATEMENT_CLASS value must be of type array, string given
4545
TypeError: Attribute value must be int for selected attribute, null given
4646
bool(true)
4747
TypeError: Attribute value must be int for selected attribute, string given

‎ext/pdo_mysql/tests/pdo_mysql_attr_errmode.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ error_reporting=E_ALL
161161
TypeError: Attribute value must be int for selected attribute, array given
162162
TypeError: Attribute value must be int for selected attribute, stdClass given
163163
TypeError: Attribute value must be int for selected attribute, string given
164-
ValueError: Error mode must be one of PDO::ERRMODE_* constants
164+
ValueError: Error mode must be one of the PDO::ERRMODE_* constants
165165

166166
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
167167

‎ext/pdo_mysql/tests/pdo_mysql_attr_statement_class.phpt

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,11 @@ array(1) {
125125
[0]=>
126126
string(12) "PDOStatement"
127127
}
128-
PDO::ATTR_STATEMENT_CLASS's value must be of type array, string given
129-
PDO::ATTR_STATEMENT_CLASS's class must be a valid class
130-
PDO::ATTR_STATEMENT_CLASS's class must be a valid class
131-
PDO::ATTR_STATEMENT_CLASS's class must be derived from PDOStatement
132-
133-
Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error: user-supplied statement class cannot have a public constructor in %s on line %d
134-
135-
Warning: PDO::setAttribute(): SQLSTATE[HY000]: General error in %s on line %d
128+
PDO::ATTR_STATEMENT_CLASS value must be of type array, string given
129+
PDO::ATTR_STATEMENT_CLASS class must be a valid class
130+
PDO::ATTR_STATEMENT_CLASS class must be a valid class
131+
PDO::ATTR_STATEMENT_CLASS class must be derived from PDOStatement
132+
TypeError: User-supplied statement class cannot have a public constructor
136133
array(2) {
137134
[0]=>
138135
string(12) "mystatement4"

0 commit comments

Comments
 (0)