Skip to content

Commit

Permalink
Fixed bug #72368
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nikic committed Dec 10, 2020
1 parent a552757 commit 9e3ba77
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 69 deletions.
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
50 changes: 25 additions & 25 deletions ext/pdo/pdo_sql_parser.re
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion ext/pdo/pdo_stmt.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
2 changes: 0 additions & 2 deletions ext/pdo/tests/bug_43130.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
43 changes: 43 additions & 0 deletions ext/pdo/tests/bug_72368.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
PDO Common: Bug #72368 (PdoStatement->execute() fails but does not throw an exception)
--SKIPIF--
<?php # vim:ft=php
if (!extension_loaded('pdo')) die('skip');
$dir = getenv('REDIR_TEST_DIR');
if (false == $dir) die('skip no driver');
require_once $dir . 'pdo_test.inc';
PDOTest::skip();
?>
--FILE--
<?php
if (getenv('REDIR_TEST_DIR') === false) putenv('REDIR_TEST_DIR='.dirname(__FILE__) . '/../../pdo/tests/');
require_once getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
$db = PDOTest::factory();
$db->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===
21 changes: 16 additions & 5 deletions ext/pdo_mysql/tests/bug41125.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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 - -
Expand Down Expand Up @@ -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 - -
Expand All @@ -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]]
Expand Down
20 changes: 7 additions & 13 deletions ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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"]=>
Expand All @@ -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]=>
Expand Down Expand Up @@ -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]=>
Expand Down
15 changes: 7 additions & 8 deletions ext/pdo_mysql/tests/pdo_mysql_prepare_emulated_anonymous.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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!
10 changes: 5 additions & 5 deletions ext/pdo_pgsql/tests/bug70313.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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

0 comments on commit 9e3ba77

Please sign in to comment.