MFH:

- Fixed bug #44173 (PDO->query() parameter parsing/checking needs an
  update)
# The changeset is longer than really needed because pdo_stmt.c was
# refactored to remove the "goto"

4 files changed, 181 insertions, 57 deletions

diff --git a/NEWS b/NEWS
index a40a2759c8..e31a703703 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,8 @@ PHP                                                                        NEWS
 - Fixed bug #47038 (Memory leak in include). (Dmitry)
 - Fixed bug #44861 (scrollable cursor don't work with pgsql). (Matteo)
 - Fixed bug #44409 (PDO::FETCH_SERIALIZE calls __construct()). (Matteo)
+- Fixed bug #44173 (PDO->query() parameter parsing/checking needs an 
+  update). (Matteo)
 - Fixed bug #42362 (HTTP status codes 204 and 304 should not be gzipped).
   (Scott, Edward Z. Yang)
 
diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c
index ef4d6a8b58..66e83ce7d0 100755
--- a/ext/pdo/pdo_dbh.c
+++ b/ext/pdo/pdo_dbh.c
@@ -1065,6 +1065,12 @@ static PHP_METHOD(PDO, query)
 	char *statement;
 	int statement_len;
 
+	/* Return a meaningful error when no parameters were passed */
+	if (!ZEND_NUM_ARGS()) {
+		zend_parse_parameters(0 TSRMLS_CC, "z|z", NULL, NULL);
+		RETURN_FALSE;
+	}
+	
 	if (FAILURE == zend_parse_parameters(1 TSRMLS_CC, "s", &statement,
 			&statement_len)) {
 		RETURN_FALSE;
@@ -1095,8 +1101,8 @@ static PHP_METHOD(PDO, query)
 	ZVAL_NULL(&stmt->lazy_object_ref);
 
 	if (dbh->methods->preparer(dbh, statement, statement_len, stmt, NULL TSRMLS_CC)) {
+		PDO_STMT_CLEAR_ERR();
 		if (ZEND_NUM_ARGS() == 1 || SUCCESS == pdo_stmt_setup_fetch_mode(INTERNAL_FUNCTION_PARAM_PASSTHRU, stmt, 1)) {
-			PDO_STMT_CLEAR_ERR();
 
 			/* now execute the statement */
 			PDO_STMT_CLEAR_ERR();
diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c
index 698ef564d8..838a235169 100755
--- a/ext/pdo/pdo_stmt.c
+++ b/ext/pdo/pdo_stmt.c
@@ -1951,6 +1951,7 @@ int pdo_stmt_setup_fetch_mode(INTERNAL_FUNCTION_PARAMETERS, pdo_stmt_t *stmt, in
 	int flags, argc = ZEND_NUM_ARGS() - skip;
 	zval ***args;
 	zend_class_entry **cep;
+	int retval;
 	
 	do_fetch_opt_finish(stmt, 1 TSRMLS_CC);
 
@@ -1973,21 +1974,27 @@ int pdo_stmt_setup_fetch_mode(INTERNAL_FUNCTION_PARAMETERS, pdo_stmt_t *stmt, in
 
 	args = safe_emalloc(ZEND_NUM_ARGS(), sizeof(zval*), 0);
 
-	if (FAILURE == zend_get_parameters_array_ex(ZEND_NUM_ARGS(), args)) {
-fail_out:
-		efree(args);
-		return FAILURE;
-	}
+	retval = zend_get_parameters_array_ex(ZEND_NUM_ARGS(), args);
 	
-	convert_to_long_ex(args[skip]);
-	mode = Z_LVAL_PP(args[skip]);
-	flags = mode & PDO_FETCH_FLAGS;
+	if (SUCCESS == retval) {
+		if (Z_TYPE_PP(args[skip]) != IS_LONG) {
+			pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "mode must be an integer" TSRMLS_CC);
+			retval = FAILURE;
+		} else {
+			mode = Z_LVAL_PP(args[skip]);
+			flags = mode & PDO_FETCH_FLAGS;
 	
-	if (!pdo_stmt_verify_mode(stmt, mode, 0 TSRMLS_CC)) {
+			retval = pdo_stmt_verify_mode(stmt, mode, 0 TSRMLS_CC);
+		}
+	}
+	
+	if (FAILURE == retval) {
+		PDO_STMT_CLEAR_ERR();
 		efree(args);
 		return FAILURE;
 	}
 
+	retval = FAILURE;
 	switch (mode & ~PDO_FETCH_FLAGS) {
 		case PDO_FETCH_USE_DEFAULT:
 		case PDO_FETCH_LAZY:
@@ -1998,89 +2005,120 @@ fail_out:
 		case PDO_FETCH_BOUND:
 		case PDO_FETCH_NAMED:
 		case PDO_FETCH_KEY_PAIR:
+			if (argc != 1) {
+				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode doesn't allow any extra arguments" TSRMLS_CC);
+			} else {
+				retval = SUCCESS;
+			}
 			break;
 
 		case PDO_FETCH_COLUMN:
 			if (argc != 2) {
-				goto fail_out;
+				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode requires the colno argument" TSRMLS_CC);
+			} else	if (Z_TYPE_PP(args[skip+1]) != IS_LONG) {
+				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "colno must be an integer" TSRMLS_CC);
+			} else {
+				stmt->fetch.column = Z_LVAL_PP(args[skip+1]);
+				retval = SUCCESS;
 			}
-			convert_to_long_ex(args[skip+1]);
-			stmt->fetch.column = Z_LVAL_PP(args[skip+1]);
 			break;
 
 		case PDO_FETCH_CLASS:
 			/* Gets its class name from 1st column */
 			if ((flags & PDO_FETCH_CLASSTYPE) == PDO_FETCH_CLASSTYPE) {
 				if (argc != 1) {
-					goto fail_out;
+					pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode doesn't allow any extra arguments" TSRMLS_CC);
+				} else {
+					stmt->fetch.cls.ce = NULL;
+					retval = SUCCESS;
 				}
-				stmt->fetch.cls.ce = NULL;
 			} else {
-				if (argc < 2 || argc > 3) {
-					goto fail_out;
-				}				
-				convert_to_string_ex(args[skip+1]);
-				
-				if (FAILURE == zend_lookup_class(Z_STRVAL_PP(args[skip+1]),
-						Z_STRLEN_PP(args[skip+1]), &cep TSRMLS_CC)) {
-					goto fail_out;
-				}
-					
-				if (!cep || !*cep) {
-					goto fail_out;
+				if (argc < 2) {
+					pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode requires the classname argument" TSRMLS_CC);
+				} else if (argc > 3) {
+					pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "too many arguments" TSRMLS_CC);
+				} else if (Z_TYPE_PP(args[skip+1]) != IS_STRING) {
+					pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "classname must be a string" TSRMLS_CC);
+				} else {
+					retval = zend_lookup_class(Z_STRVAL_PP(args[skip+1]),
+						Z_STRLEN_PP(args[skip+1]), &cep TSRMLS_CC);
+
+					if (SUCCESS == retval && cep && *cep) {
+						stmt->fetch.cls.ce = *cep;
+					}
 				}
-				
-				stmt->fetch.cls.ce = *cep;				
 			}
 
-			stmt->fetch.cls.ctor_args = NULL;
+			if (SUCCESS == retval) {
+				stmt->fetch.cls.ctor_args = NULL;
 #ifdef ilia_0 /* we'll only need this when we have persistent statements, if ever */
-			if (stmt->dbh->is_persistent) {
-				php_error_docref(NULL TSRMLS_CC, E_WARNING, "PHP might crash if you don't call $stmt->setFetchMode() to reset to defaults on this persistent statement.  This will be fixed in a later release");
-			}
+				if (stmt->dbh->is_persistent) {
+					php_error_docref(NULL TSRMLS_CC, E_WARNING, "PHP might crash if you don't call $stmt->setFetchMode() to reset to defaults on this persistent statement.  This will be fixed in a later release");
+				}
 #endif
-			if (argc == 3) {
-				if (Z_TYPE_PP(args[skip+2]) != IS_NULL && Z_TYPE_PP(args[skip+2]) != IS_ARRAY) {
-					pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "ctor_args must be either NULL or an array" TSRMLS_CC);
-				} else if (Z_TYPE_PP(args[skip+2]) == IS_ARRAY && zend_hash_num_elements(Z_ARRVAL_PP(args[skip+2]))) {
-					ALLOC_ZVAL(stmt->fetch.cls.ctor_args);
-					*stmt->fetch.cls.ctor_args = **args[skip+2];
-					zval_copy_ctor(stmt->fetch.cls.ctor_args);
+				if (argc == 3) {
+					if (Z_TYPE_PP(args[skip+2]) != IS_NULL && Z_TYPE_PP(args[skip+2]) != IS_ARRAY) {
+						pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "ctor_args must be either NULL or an array" TSRMLS_CC);
+						retval = FAILURE;
+					} else if (Z_TYPE_PP(args[skip+2]) == IS_ARRAY && zend_hash_num_elements(Z_ARRVAL_PP(args[skip+2]))) {
+						ALLOC_ZVAL(stmt->fetch.cls.ctor_args);
+						*stmt->fetch.cls.ctor_args = **args[skip+2];
+						zval_copy_ctor(stmt->fetch.cls.ctor_args);
+					}
+				}
+
+				if (SUCCESS == retval) {
+					do_fetch_class_prepare(stmt TSRMLS_CC);
 				}
 			}
 			
-			do_fetch_class_prepare(stmt TSRMLS_CC);
 			break;
 
 		case PDO_FETCH_INTO:
 			if (argc != 2) {
-				goto fail_out;
-			}
-			if (Z_TYPE_PP(args[skip+1]) != IS_OBJECT) {
-				goto fail_out;
+				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "fetch mode requires the object parameter" TSRMLS_CC);
+			} else if (Z_TYPE_PP(args[skip+1]) != IS_OBJECT) {
+				pdo_raise_impl_error(stmt->dbh, stmt, "HY000", "object must be an object" TSRMLS_CC);
+			} else {
+				retval = SUCCESS;
 			}
+			
+			if (SUCCESS == retval) {
 #ifdef ilia_0 /* we'll only need this when we have persistent statements, if ever */
-			if (stmt->dbh->is_persistent) {
-				php_error_docref(NULL TSRMLS_CC, E_WARNING, "PHP might crash if you don't call $stmt->setFetchMode() to reset to defaults on this persistent statement.  This will be fixed in a later release");
-			}
+				if (stmt->dbh->is_persistent) {
+					php_error_docref(NULL TSRMLS_CC, E_WARNING, "PHP might crash if you don't call $stmt->setFetchMode() to reset to defaults on this persistent statement.  This will be fixed in a later release");
+				}
 #endif	
-			MAKE_STD_ZVAL(stmt->fetch.into);
+				MAKE_STD_ZVAL(stmt->fetch.into);
 
-			Z_TYPE_P(stmt->fetch.into) = IS_OBJECT;
-			Z_OBJ_HANDLE_P(stmt->fetch.into) = Z_OBJ_HANDLE_PP(args[skip+1]);
-			Z_OBJ_HT_P(stmt->fetch.into) = Z_OBJ_HT_PP(args[skip+1]);
-			zend_objects_store_add_ref(stmt->fetch.into TSRMLS_CC);
+				Z_TYPE_P(stmt->fetch.into) = IS_OBJECT;
+				Z_OBJ_HANDLE_P(stmt->fetch.into) = Z_OBJ_HANDLE_PP(args[skip+1]);
+				Z_OBJ_HT_P(stmt->fetch.into) = Z_OBJ_HT_PP(args[skip+1]);
+				zend_objects_store_add_ref(stmt->fetch.into TSRMLS_CC);
+			}
+			
 			break;
 		
 		default:
 			pdo_raise_impl_error(stmt->dbh, stmt, "22003", "Invalid fetch mode specified" TSRMLS_CC);
-			goto fail_out;
 	}
 
-	stmt->default_fetch_type = mode;
-	efree(args);
+	if (SUCCESS == retval) {
+		stmt->default_fetch_type = mode;
+	}
 
-	return SUCCESS;
+	/*
+	 * PDO error (if any) has already been raised at this point.
+	 *
+	 * The error_code is cleared, otherwise the caller will read the
+	 * last error message from the driver.
+	 *
+	 */
+	PDO_STMT_CLEAR_ERR();
+
+	efree(args);
+		
+	return retval;
 }
    
 static PHP_METHOD(PDOStatement, setFetchMode)
diff --git a/ext/pdo/tests/bug_44173.phpt b/ext/pdo/tests/bug_44173.phpt
new file mode 100644
index 0000000000..f13abaa5d5
--- /dev/null
+++ b/ext/pdo/tests/bug_44173.phpt
@@ -0,0 +1,78 @@
+--TEST--
+PDO Common: Bug #44173 (PDO->query() parameter parsing/checking needs an update)
+--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 getenv('REDIR_TEST_DIR') . 'pdo_test.inc';
+$db = PDOTest::factory();
+
+$db->exec("CREATE TABLE test (x int)");
+$db->exec("INSERT INTO test VALUES (1)");
+
+
+// Bug entry [1]
+$stmt = $db->query();
+var_dump($stmt);
+
+
+// Bug entry [2] -- 1 is PDO::FETCH_LAZY
+$stmt = $db->query("SELECT * FROM test", PDO::FETCH_LAZY, 0, 0);
+var_dump($stmt);
+
+
+// Bug entry [3]
+$stmt = $db->query("SELECT * FROM test", 'abc');
+var_dump($stmt);
+
+
+// Bug entry [4]
+$stmt = $db->query("SELECT * FROM test", PDO::FETCH_CLASS, 0, 0, 0);
+var_dump($stmt);
+
+
+// Bug entry [5]
+$stmt = $db->query("SELECT * FROM test", PDO::FETCH_INTO);
+var_dump($stmt);
+
+
+// Bug entry [6]
+$stmt = $db->query("SELECT * FROM test", PDO::FETCH_COLUMN);
+var_dump($stmt);
+
+
+// Bug entry [7]
+$stmt = $db->query("SELECT * FROM test", PDO::FETCH_CLASS);
+var_dump($stmt);
+
+
+?>
+--EXPECTF--
+Warning: PDO::query() expects at least 1 parameter, 0 given in %s
+bool(false)
+
+Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode doesn't allow any extra arguments in %s
+bool(false)
+
+Warning: PDO::query(): SQLSTATE[HY000]: General error: mode must be an integer in %s
+bool(false)
+
+Warning: PDO::query(): SQLSTATE[HY000]: General error: too many arguments in %s
+bool(false)
+
+Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode requires the object parameter in %s
+bool(false)
+
+Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode requires the colno argument in %s
+bool(false)
+
+Warning: PDO::query(): SQLSTATE[HY000]: General error: fetch mode requires the classname argument in %s
+bool(false)
+