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.

11 files changed, 113 insertions, 69 deletions

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--
+<?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===
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