From 9e3ba775b7de7d7647c488beb9e302d03690f955 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 10 Dec 2020 16:52:17 +0100 Subject: Fixed bug #72368 Generate a param count mismatch error even if the query contains no placeholders. Additionally we shouldn't HANDLE errors from pdo_parse_params, which are always reported via raise_impl_error. Doing so results in duplicate error messages. --- NEWS | 2 + ext/pdo/pdo_sql_parser.re | 50 +++++++++++----------- ext/pdo/pdo_stmt.c | 1 - ext/pdo/tests/bug_43130.phpt | 2 - ext/pdo/tests/bug_72368.phpt | 43 +++++++++++++++++++ ext/pdo_mysql/tests/bug41125.phpt | 21 ++++++--- .../tests/pdo_mysql_prepare_emulated.phpt | 20 +++------ .../pdo_mysql_prepare_emulated_anonymous.phpt | 15 +++---- ...ql_prepare_emulated_placeholder_everywhere.phpt | 2 - ...pdo_mysql_prepare_native_named_placeholder.phpt | 16 +++---- ext/pdo_pgsql/tests/bug70313.phpt | 10 ++--- 11 files changed, 113 insertions(+), 69 deletions(-) create mode 100644 ext/pdo/tests/bug_72368.phpt diff --git a/NEWS b/NEWS index 536b028fd9..f8d543934f 100644 --- a/NEWS +++ b/NEWS @@ -54,6 +54,8 @@ PHP NEWS . Fixed bug #79872 (Can't execute query with pending result sets). (Nikita) . Fixed bug #79131 (PDO does not throw an exception when parameter values are missing). (Nikita) + . Fixed bug #72368 (PdoStatement->execute() fails but does not throw an + exception). (Nikita) - Phar: . Fixed bug #73809 (Phar Zip parse crash - mmap fail). (cmb) diff --git a/ext/pdo/pdo_sql_parser.re b/ext/pdo/pdo_sql_parser.re index ee0ce7168d..04a4118fc4 100644 --- a/ext/pdo/pdo_sql_parser.re +++ b/ext/pdo/pdo_sql_parser.re @@ -143,11 +143,6 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, const char *inquery, size_t inque } } - if (!placeholders) { - /* nothing to do; good! */ - return 0; - } - /* did the query make sense to me? */ if (query_type == (PDO_PLACEHOLDER_NAMED|PDO_PLACEHOLDER_POSITIONAL)) { /* they mixed both types; punt */ @@ -156,6 +151,31 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, const char *inquery, size_t inque goto clean_up; } + params = stmt->bound_params; + if (stmt->supports_placeholders == PDO_PLACEHOLDER_NONE && params && bindno != zend_hash_num_elements(params)) { + /* extra bit of validation for instances when same params are bound more than once */ + if (query_type != PDO_PLACEHOLDER_POSITIONAL && bindno > zend_hash_num_elements(params)) { + int ok = 1; + for (plc = placeholders; plc; plc = plc->next) { + if ((param = zend_hash_str_find_ptr(params, plc->pos, plc->len)) == NULL) { + ok = 0; + break; + } + } + if (ok) { + goto safe; + } + } + pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "number of bound variables does not match number of tokens"); + ret = -1; + goto clean_up; + } + + if (!placeholders) { + /* nothing to do; good! */ + return 0; + } + if (stmt->supports_placeholders == query_type && !stmt->named_rewrite_template) { /* query matches native syntax */ if (escapes) { @@ -176,26 +196,6 @@ PDO_API int pdo_parse_params(pdo_stmt_t *stmt, const char *inquery, size_t inque query_type = PDO_PLACEHOLDER_POSITIONAL; } - params = stmt->bound_params; - - if (bindno && stmt->supports_placeholders == PDO_PLACEHOLDER_NONE && params && bindno != zend_hash_num_elements(params)) { - /* extra bit of validation for instances when same params are bound more than once */ - if (query_type != PDO_PLACEHOLDER_POSITIONAL && bindno > zend_hash_num_elements(params)) { - int ok = 1; - for (plc = placeholders; plc; plc = plc->next) { - if ((param = zend_hash_str_find_ptr(params, plc->pos, plc->len)) == NULL) { - ok = 0; - break; - } - } - if (ok) { - goto safe; - } - } - pdo_raise_impl_error(stmt->dbh, stmt, "HY093", "number of bound variables does not match number of tokens"); - ret = -1; - goto clean_up; - } safe: /* what are we going to do ? */ if (stmt->supports_placeholders == PDO_PLACEHOLDER_NONE) { diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 68ec99160a..a172ae27c5 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -458,7 +458,6 @@ PHP_METHOD(PDOStatement, execute) ret = 1; } else if (ret == -1) { /* something broke */ - PDO_HANDLE_STMT_ERR(); RETURN_FALSE; } } else if (!dispatch_param_event(stmt, PDO_PARAM_EVT_EXEC_PRE)) { diff --git a/ext/pdo/tests/bug_43130.phpt b/ext/pdo/tests/bug_43130.phpt index bf65bf0513..4615038e3f 100644 --- a/ext/pdo/tests/bug_43130.phpt +++ b/ext/pdo/tests/bug_43130.phpt @@ -36,6 +36,4 @@ var_dump($stmt->fetch(PDO::FETCH_COLUMN)); ?> --EXPECTF-- Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: parameter was not defined in %s on line %d - -Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number in %s on line %d bool(false) diff --git a/ext/pdo/tests/bug_72368.phpt b/ext/pdo/tests/bug_72368.phpt new file mode 100644 index 0000000000..db8d4993a8 --- /dev/null +++ b/ext/pdo/tests/bug_72368.phpt @@ -0,0 +1,43 @@ +--TEST-- +PDO Common: Bug #72368 (PdoStatement->execute() fails but does not throw an exception) +--SKIPIF-- + +--FILE-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + +$params = [":bar" => 1]; +$sql = "SELECT 1"; + +$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false); +try { + $stmt = $db->prepare($sql); + var_dump($stmt->execute($params)); +} catch (PDOException $e) { + var_dump('ERR'); +} + + +$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, true); +try { + $stmt = $db->prepare($sql); + var_dump($stmt->execute($params)); +} catch (PDOException $e) { + var_dump('ERR'); +} + +?> +===DONE=== +--EXPECT-- +string(3) "ERR" +string(3) "ERR" +===DONE=== diff --git a/ext/pdo_mysql/tests/bug41125.phpt b/ext/pdo_mysql/tests/bug41125.phpt index 63ff7df2f3..cc5edba6fa 100644 --- a/ext/pdo_mysql/tests/bug41125.phpt +++ b/ext/pdo_mysql/tests/bug41125.phpt @@ -83,10 +83,12 @@ foreach ($queries as $k => $query) { } ?> ---EXPECT-- +--EXPECTF-- 1 00000 - - ------------------------------------------------------- + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d [1] Query: [[SELECT 1 FROM DUAL WHERE 1 = '?\'\'']] 00000 - - @@ -123,18 +125,24 @@ O'\0 1 00000 - - -------- + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d [5] Query: [[SELECT 'a', 'b\'' FROM DUAL WHERE '''' LIKE '\'' AND 1]] -a - b' + 00000 - - -------- + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d [6] Query: [[SELECT 'a''', '\'b\'' FROM DUAL WHERE '''' LIKE '\'' AND 1]] -a' - 'b' + 00000 - - -------- [7] Query: [[SELECT UPPER(:id) FROM DUAL WHERE '1']] 1 00000 - - -------- + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d [8] Query: [[SELECT 1 FROM DUAL WHERE '\'']] 00000 - - @@ -147,13 +155,16 @@ a' - 'b' 00000 - - -------- + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d [11] Query: [[SELECT 1 FROM DUAL WHERE '\'' = '''']] -1 + 00000 - - -------- + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d [12] Query: [[SELECT '\n' '1 FROM DUAL WHERE '''' and :id']] -1 FROM DUAL WHERE '' and :id 00000 - - -------- [13] Query: [[SELECT 1 'FROM DUAL WHERE :id AND '''' = '''' OR 1 = 1 AND ':id]] diff --git a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt index 808baffae3..b23d697efa 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt @@ -326,7 +326,7 @@ require __DIR__ . '/mysql_pdo_test.inc'; $db = MySQLPDOTest::factory(); $db->exec('DROP TABLE IF EXISTS test'); ?> ---EXPECT-- +--EXPECTF-- PDO::prepare(): Argument #1 ($query) cannot be empty array(1) { ["one"]=> @@ -339,12 +339,9 @@ array(1) { string(12) ":placeholder" } } -array(1) { - [0]=> - array(1) { - ["label"]=> - string(12) ":placeholder" - } + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d +array(0) { } array(2) { [0]=> @@ -381,12 +378,9 @@ array(1) { string(1) "?" } } -array(1) { - [0]=> - array(1) { - ["label"]=> - string(1) "?" - } + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d +array(0) { } array(2) { [0]=> diff --git a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_anonymous.phpt b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_anonymous.phpt index 5d403c0caa..b652a08ea2 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_anonymous.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_anonymous.phpt @@ -65,14 +65,13 @@ $db = MySQLPDOTest::factory(); $db->exec('DROP TABLE IF EXISTS test'); ?> --EXPECTF-- -array(1) { - [0]=> - array(2) { - ["id"]=> - string(1) "1" - ["label"]=> - string(1) "?" - } +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d +[003] Execute has failed, 'HY093' array ( + 0 => 'HY093', + 1 => NULL, + 2 => NULL, +) +array(0) { } now the same with native PS diff --git a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_placeholder_everywhere.phpt b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_placeholder_everywhere.phpt index e47b03b999..abd809539b 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_placeholder_everywhere.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_placeholder_everywhere.phpt @@ -72,8 +72,6 @@ array(0) { now the same with emulated PS Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d - -Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number in %s on line 33 [005] Execute has failed, 'HY093' array ( 0 => 'HY093', 1 => NULL, diff --git a/ext/pdo_mysql/tests/pdo_mysql_prepare_native_named_placeholder.phpt b/ext/pdo_mysql/tests/pdo_mysql_prepare_native_named_placeholder.phpt index ea4d097809..35658d5529 100644 --- a/ext/pdo_mysql/tests/pdo_mysql_prepare_native_named_placeholder.phpt +++ b/ext/pdo_mysql/tests/pdo_mysql_prepare_native_named_placeholder.phpt @@ -81,13 +81,13 @@ Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number in % ) array(0) { } -array(1) { - [0]=> - array(2) { - ["id"]=> - string(3) "101" - ["label"]=> - string(12) ":placeholder" - } + +Warning: PDOStatement::execute(): SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens in %s on line %d +[005] Execute has failed, 'HY093' array ( + 0 => 'HY093', + 1 => NULL, + 2 => NULL, +) +array(0) { } done! diff --git a/ext/pdo_pgsql/tests/bug70313.phpt b/ext/pdo_pgsql/tests/bug70313.phpt index 07c7f68ab3..e6f9e4593e 100644 --- a/ext/pdo_pgsql/tests/bug70313.phpt +++ b/ext/pdo_pgsql/tests/bug70313.phpt @@ -19,7 +19,7 @@ try { $stmt->execute([1]); } catch (PDOException $e) { - var_dump($e->getCode()); + echo $e->getMessage(), "\n"; } $db->setAttribute(PDO::ATTR_EMULATE_PREPARES, true); @@ -28,10 +28,10 @@ try { $stmt->execute([1]); } catch (PDOException $e) { - var_dump($e->getCode()); + echo $e->getMessage(), "\n"; } ?> ---EXPECT-- -string(5) "42601" -string(5) "42601" +--EXPECTF-- +SQLSTATE[42601]: Syntax error: %A +SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens -- cgit v1.2.1