- 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"

diff --git a/ext/pdo/pdo_dbh.c b/ext/pdo/pdo_dbh.c
index 1dc4f133d2702efae37f8d45601c65e23942c60d..77ef136ba6a03882ae3d5495ab789bb26fa5b94b 100755 (executable)
--- a/ext/pdo/pdo_dbh.c
+++ b/ext/pdo/pdo_dbh.c
@@ -1066,6 +1066,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;
@@ -1096,8 +1102,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 5b02af585b8df95d5c872979010376466aad5745..59905c7f9a0e85c85b096604eaf9bf1aec63d756 100755 (executable)
--- a/ext/pdo/pdo_stmt.c
+++ b/ext/pdo/pdo_stmt.c
@@ -1945,6 +1945,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);
 
@@ -1967,21 +1968,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:
@@ -1992,93 +1999,122 @@ 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");
-                       }
-#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 (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);
+                                               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 (file)
index 0000000..f13abaa
--- /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)
+