From 2b6c9b68bbc131e5d86bcb736debcea178aab02a 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] 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. Closes GH-17908. --- NEWS | 4 ++++ UPGRADING | 4 ++++ ext/mysqli/mysqli_api.c | 5 +---- ext/mysqli/mysqli_nonapi.c | 15 ++++++++++++++- ext/mysqli/tests/gh17900.phpt | 16 ++++++++++++++++ .../tests/mysqli_incomplete_initialization.phpt | 4 ++-- 6 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 ext/mysqli/tests/gh17900.phpt diff --git a/NEWS b/NEWS index c9a77af4b63..675ac037456 100644 --- a/NEWS +++ b/NEWS @@ -56,6 +56,10 @@ PHP NEWS . IntlDateFormatter::setTimeZone()/datefmt_set_timezone() throws an exception with uninitialised classes or clone failure. (David Carlier) +- MySQLi: + . Fixed bugs GH-17900 and GH-8084 (calling mysqli::__construct twice). + (nielsdos) + - MySQLnd: . Added mysqlnd.collect_memory_statistics to ini quick reference. (hauk92) diff --git a/UPGRADING b/UPGRADING index 708303b3259..6b4f6c33584 100644 --- a/UPGRADING +++ b/UPGRADING @@ -44,6 +44,10 @@ PHP 8.5 UPGRADE NOTES . ldap_get_option() and ldap_set_option() now throw a ValueError when passing an invalid option. +- MySQLi: + . Calling the mysqli constructor on an already-constructed object + is now no longer possible and throws an Error. + - PCNTL: . pcntl_exec() now throws ValueErrors when entries of the $args parameter contain null bytes. diff --git a/ext/mysqli/mysqli_api.c b/ext/mysqli/mysqli_api.c index 2bc33e4ad67..5e2645740b2 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 365c4891a09..e0e14eeccbc 100644 --- a/ext/mysqli/mysqli_nonapi.c +++ b/ext/mysqli/mysqli_nonapi.c @@ -72,7 +72,14 @@ void mysqli_common_connect(INTERNAL_FUNCTION_PARAMETERS, bool is_real_connect, b } #endif - if (getThis() && !ZEND_NUM_ARGS() && in_ctor) { + 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; } @@ -84,6 +91,11 @@ 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) { ZEND_ASSERT(instanceof_function(Z_OBJCE_P(object), mysqli_link_class_entry)); mysqli_resource = (Z_MYSQLI_P(object))->ptr; @@ -325,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); } /* }}} */ diff --git a/ext/mysqli/tests/gh17900.phpt b/ext/mysqli/tests/gh17900.phpt new file mode 100644 index 00000000000..ed099fa7e85 --- /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 51a4d44c72b..a5450bd105f 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