From 4205c31dff1b31dcf7a6d3608982d7370b97b183 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 23 Feb 2025 17:42:04 +0100 Subject: [PATCH 1/2] Fix GH-17900 and GH-8084 Calling the constructor twice has no real world benefit. Block it to fix these two issues. We also clean up the constructor code a bit: - `in_ctor` implies `object` exist. - We surround the instance check with ZEND_DEBUG to avoid a runtime penalty. Closes GH-17900. Closes GH-8084. --- ext/mysqli/mysqli_nonapi.c | 9 ++++++++- ext/mysqli/tests/gh17900.phpt | 16 ++++++++++++++++ .../tests/mysqli_incomplete_initialization.phpt | 4 ++-- 3 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 ext/mysqli/tests/gh17900.phpt diff --git a/ext/mysqli/mysqli_nonapi.c b/ext/mysqli/mysqli_nonapi.c index 365c4891a09a7..7e4c4ba1ba0a0 100644 --- a/ext/mysqli/mysqli_nonapi.c +++ b/ext/mysqli/mysqli_nonapi.c @@ -72,7 +72,12 @@ void mysqli_common_connect(INTERNAL_FUNCTION_PARAMETERS, bool is_real_connect, b } #endif - if (getThis() && !ZEND_NUM_ARGS() && in_ctor) { + if (UNEXPECTED(in_ctor && (Z_MYSQLI_P(object))->ptr)) { + zend_throw_error(NULL, "Cannot call constructor twice"); + return; + } + + if (in_ctor && !ZEND_NUM_ARGS()) { php_mysqli_init(INTERNAL_FUNCTION_PARAM_PASSTHRU, in_ctor); return; } @@ -85,7 +90,9 @@ void mysqli_common_connect(INTERNAL_FUNCTION_PARAMETERS, bool is_real_connect, b } if (object) { +#if ZEND_DEBUG ZEND_ASSERT(instanceof_function(Z_OBJCE_P(object), mysqli_link_class_entry)); +#endif mysqli_resource = (Z_MYSQLI_P(object))->ptr; if (mysqli_resource && mysqli_resource->ptr) { mysql = (MY_MYSQL*) mysqli_resource->ptr; diff --git a/ext/mysqli/tests/gh17900.phpt b/ext/mysqli/tests/gh17900.phpt new file mode 100644 index 0000000000000..ed099fa7e8529 --- /dev/null +++ b/ext/mysqli/tests/gh17900.phpt @@ -0,0 +1,16 @@ +--TEST-- +GH-17900 (Assertion failure ext/mysqli/mysqli_prop.c) +--EXTENSIONS-- +mysqli +--FILE-- +__construct('doesnotexist'); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +?> +--EXPECT-- +Cannot call constructor twice diff --git a/ext/mysqli/tests/mysqli_incomplete_initialization.phpt b/ext/mysqli/tests/mysqli_incomplete_initialization.phpt index 51a4d44c72b2f..a5450bd105f44 100644 --- a/ext/mysqli/tests/mysqli_incomplete_initialization.phpt +++ b/ext/mysqli/tests/mysqli_incomplete_initialization.phpt @@ -12,8 +12,8 @@ $mysqli->close(); ?> --EXPECTF-- -Fatal error: Uncaught Error: mysqli object is not fully initialized in %s:%d +Fatal error: Uncaught Error: Cannot call constructor twice in %s:%d Stack trace: -#0 %s(%d): mysqli->close() +#0 %s(%d): mysqli->__construct('doesnotexist') #1 {main} thrown in %s on line %d From 63b348d959504bf468bd38536ee36d0b3eebce94 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 23 Feb 2025 22:44:01 +0100 Subject: [PATCH 2/2] Review --- ext/mysqli/mysqli_api.c | 5 +---- ext/mysqli/mysqli_nonapi.c | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/ext/mysqli/mysqli_api.c b/ext/mysqli/mysqli_api.c index 2bc33e4ad673c..5e2645740b26e 100644 --- a/ext/mysqli/mysqli_api.c +++ b/ext/mysqli/mysqli_api.c @@ -969,10 +969,6 @@ void php_mysqli_init(INTERNAL_FUNCTION_PARAMETERS, bool is_method) MYSQLI_RESOURCE *mysqli_resource; MY_MYSQL *mysql; - if (zend_parse_parameters_none() == FAILURE) { - RETURN_THROWS(); - } - if (is_method && (Z_MYSQLI_P(getThis()))->ptr) { return; } @@ -1004,6 +1000,7 @@ void php_mysqli_init(INTERNAL_FUNCTION_PARAMETERS, bool is_method) /* {{{ Initialize mysqli and return a resource for use with mysql_real_connect */ PHP_FUNCTION(mysqli_init) { + ZEND_PARSE_PARAMETERS_NONE(); php_mysqli_init(INTERNAL_FUNCTION_PARAM_PASSTHRU, false); } /* }}} */ diff --git a/ext/mysqli/mysqli_nonapi.c b/ext/mysqli/mysqli_nonapi.c index 7e4c4ba1ba0a0..2ae6a2c3b3129 100644 --- a/ext/mysqli/mysqli_nonapi.c +++ b/ext/mysqli/mysqli_nonapi.c @@ -72,12 +72,14 @@ void mysqli_common_connect(INTERNAL_FUNCTION_PARAMETERS, bool is_real_connect, b } #endif - if (UNEXPECTED(in_ctor && (Z_MYSQLI_P(object))->ptr)) { - zend_throw_error(NULL, "Cannot call constructor twice"); - return; - } - if (in_ctor && !ZEND_NUM_ARGS()) { + ZEND_PARSE_PARAMETERS_NONE(); + + if (UNEXPECTED(Z_MYSQLI_P(object)->ptr)) { + zend_throw_error(NULL, "Cannot call constructor twice"); + return; + } + php_mysqli_init(INTERNAL_FUNCTION_PARAM_PASSTHRU, in_ctor); return; } @@ -89,10 +91,13 @@ void mysqli_common_connect(INTERNAL_FUNCTION_PARAMETERS, bool is_real_connect, b RETURN_THROWS(); } + if (UNEXPECTED(in_ctor && Z_MYSQLI_P(object)->ptr)) { + zend_throw_error(NULL, "Cannot call constructor twice"); + return; + } + if (object) { -#if ZEND_DEBUG ZEND_ASSERT(instanceof_function(Z_OBJCE_P(object), mysqli_link_class_entry)); -#endif mysqli_resource = (Z_MYSQLI_P(object))->ptr; if (mysqli_resource && mysqli_resource->ptr) { mysql = (MY_MYSQL*) mysqli_resource->ptr; @@ -332,6 +337,7 @@ PHP_METHOD(mysqli, __construct) /* {{{ Initialize mysqli and return a resource for use with mysql_real_connect */ PHP_METHOD(mysqli, init) { + ZEND_PARSE_PARAMETERS_NONE(); php_mysqli_init(INTERNAL_FUNCTION_PARAM_PASSTHRU, true); } /* }}} */