From 5aa6d5ad002c2a854271728ccd2ecd1973442e5e Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sun, 2 Jun 2024 16:36:11 +0100 Subject: [PATCH 01/14] Zend: Add object_init_with_constructor() API This will instantiate the object and execute its constructor with the given parameters. --- Zend/zend_API.c | 18 ++++++++++++++++++ Zend/zend_API.h | 1 + 2 files changed, 19 insertions(+) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index ae7a06f011761..4f558ad83b48c 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1846,6 +1846,24 @@ ZEND_API zend_result object_init_ex(zval *arg, zend_class_entry *class_type) /* } /* }}} */ +ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *class_type, uint32_t param_count, zval *params) /* {{{ */ +{ + zend_result status = _object_and_properties_init(arg, class_type, NULL); + if (UNEXPECTED(status == FAILURE)) { + return FAILURE; + } + /* A constructor does not return a value */ + zend_call_known_instance_method( + class_type->constructor, + Z_OBJ_P(arg), + /* retval */ NULL, + param_count, + params + ); + return SUCCESS; +} +/* }}} */ + ZEND_API void object_init(zval *arg) /* {{{ */ { ZVAL_OBJ(arg, zend_objects_new(zend_standard_class_def)); diff --git a/Zend/zend_API.h b/Zend/zend_API.h index 90556fcde4245..849177c41f5fd 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -537,6 +537,7 @@ ZEND_API const char *zend_get_type_by_const(int type); #define array_init_size(arg, size) ZVAL_ARR((arg), zend_new_array(size)) ZEND_API void object_init(zval *arg); ZEND_API zend_result object_init_ex(zval *arg, zend_class_entry *ce); +ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *class_type, uint32_t param_count, zval *params); ZEND_API zend_result object_and_properties_init(zval *arg, zend_class_entry *ce, HashTable *properties); ZEND_API void object_properties_init(zend_object *object, zend_class_entry *class_type); ZEND_API void object_properties_init_ex(zend_object *object, HashTable *properties); From b7021397e2c460f99b6d33b9a496f44bb9d3369b Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 00:33:59 +0100 Subject: [PATCH 02/14] Support named arguments and return failure on exception --- Zend/zend_API.c | 20 +++++++++++++++----- Zend/zend_API.h | 2 +- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 4f558ad83b48c..12bd36655f01f 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1846,21 +1846,31 @@ ZEND_API zend_result object_init_ex(zval *arg, zend_class_entry *class_type) /* } /* }}} */ -ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *class_type, uint32_t param_count, zval *params) /* {{{ */ +ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *class_type, uint32_t param_count, zval *params, HashTable *named_params) /* {{{ */ { zend_result status = _object_and_properties_init(arg, class_type, NULL); if (UNEXPECTED(status == FAILURE)) { return FAILURE; } - /* A constructor does not return a value */ - zend_call_known_instance_method( + /* A constructor does not return a value, however if an exception is thrown + * zend_call_known_function() will set the retval to IS_UNDEF */ + zval retval; + ZVAL_UNDEF(&retval); + zend_call_known_function( class_type->constructor, Z_OBJ_P(arg), + class_type, /* retval */ NULL, param_count, - params + params, + named_params ); - return SUCCESS; + if (Z_TYPE(retval) == IS_UNDEF) { + return FAILURE; + } else { + zval_ptr_dtor(&retval); + return SUCCESS; + } } /* }}} */ diff --git a/Zend/zend_API.h b/Zend/zend_API.h index 849177c41f5fd..ab67dd5717e69 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -537,7 +537,7 @@ ZEND_API const char *zend_get_type_by_const(int type); #define array_init_size(arg, size) ZVAL_ARR((arg), zend_new_array(size)) ZEND_API void object_init(zval *arg); ZEND_API zend_result object_init_ex(zval *arg, zend_class_entry *ce); -ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *class_type, uint32_t param_count, zval *params); +ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *class_type, uint32_t param_count, zval *params, HashTable *named_params); ZEND_API zend_result object_and_properties_init(zval *arg, zend_class_entry *ce, HashTable *properties); ZEND_API void object_properties_init(zend_object *object, zend_class_entry *class_type); ZEND_API void object_properties_init_ex(zend_object *object, HashTable *properties); From 6a2ef11515f8f499278fcf5dd2f8d65e2cabdfbe Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 01:21:24 +0100 Subject: [PATCH 03/14] Test new API and fix issues encountered in testing --- Zend/zend_API.c | 15 ++- ext/zend_test/test.c | 20 ++++ ext/zend_test/test.stub.php | 2 + ext/zend_test/test_arginfo.h | 9 +- .../zend_object_init_with_constructor.phpt | 98 +++++++++++++++++++ 5 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 ext/zend_test/tests/zend_object_init_with_constructor.phpt diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 12bd36655f01f..7d0111af09ee3 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1852,15 +1852,19 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c if (UNEXPECTED(status == FAILURE)) { return FAILURE; } - /* A constructor does not return a value, however if an exception is thrown + zend_object * obj = Z_OBJ_P(arg); + zend_function *constructor = obj->handlers->get_constructor(obj); + if (UNEXPECTED(constructor == NULL)) { + return FAILURE; + } + /* A constructor should not return a value, however if an exception is thrown * zend_call_known_function() will set the retval to IS_UNDEF */ zval retval; - ZVAL_UNDEF(&retval); zend_call_known_function( - class_type->constructor, - Z_OBJ_P(arg), + constructor, + obj, class_type, - /* retval */ NULL, + /* retval */ &retval, param_count, params, named_params @@ -1868,6 +1872,7 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c if (Z_TYPE(retval) == IS_UNDEF) { return FAILURE; } else { + /* Unlikely, but user constructors may return any value they want */ zval_ptr_dtor(&retval); return SUCCESS; } diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 7eea02cd07d34..4632158e294fd 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -460,6 +460,26 @@ static ZEND_FUNCTION(zend_call_method) zend_call_method(obj, ce, NULL, ZSTR_VAL(method_name), ZSTR_LEN(method_name), return_value, argc - 2, arg1, arg2); } +/* Instantiate a class and run the constructor via object_init_with_constructor */ +static ZEND_FUNCTION(zend_object_init_with_constructor) +{ + zend_class_entry *ce = NULL; + zval *args; + uint32_t num_args; + HashTable *named_args; + + ZEND_PARSE_PARAMETERS_START(1, -1) + Z_PARAM_CLASS(ce) + Z_PARAM_VARIADIC_WITH_NAMED(args, num_args, named_args) + ZEND_PARSE_PARAMETERS_END(); + + zend_result status = object_init_with_constructor(return_value, ce, num_args, args, named_args); + if (status == FAILURE) { + RETURN_THROWS(); + } + ZEND_ASSERT(!EG(exception)); +} + static ZEND_FUNCTION(zend_get_unit_enum) { ZEND_PARSE_PARAMETERS_NONE(); diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 5c1bed2847822..8930048b4c20f 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -247,6 +247,8 @@ function zend_get_current_func_name(): string {} function zend_call_method(object|string $obj_or_class, string $method, mixed $arg1 = UNKNOWN, mixed $arg2 = UNKNOWN): mixed {} + function zend_object_init_with_constructor(string $class, mixed ...$args): mixed {} + function zend_test_zend_ini_parse_quantity(string $str): int {} function zend_test_zend_ini_parse_uquantity(string $str): int {} diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index 32229d9d9d323..90740a7a79fdb 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 53027610adee7e1c675b1c1b38886100ce4e63ef */ + * Stub hash: 35c11b9781669cff5ad72aa78b9b3732f7b827f1 */ ZEND_STATIC_ASSERT(PHP_VERSION_ID >= 80000, "test_arginfo.h only supports PHP version ID 80000 or newer, " "but it is included on an older PHP version"); @@ -100,6 +100,11 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_call_method, 0, 2, IS_MIXED ZEND_ARG_TYPE_INFO(0, arg2, IS_MIXED, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_object_init_with_constructor, 0, 1, IS_MIXED, 0) + ZEND_ARG_TYPE_INFO(0, class, IS_STRING, 0) + ZEND_ARG_VARIADIC_TYPE_INFO(0, args, IS_MIXED, 0) +ZEND_END_ARG_INFO() + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_zend_ini_parse_quantity, 0, 1, IS_LONG, 0) ZEND_ARG_TYPE_INFO(0, str, IS_STRING, 0) ZEND_END_ARG_INFO() @@ -255,6 +260,7 @@ static ZEND_FUNCTION(zend_get_unit_enum); static ZEND_FUNCTION(zend_test_parameter_with_attribute); static ZEND_FUNCTION(zend_get_current_func_name); static ZEND_FUNCTION(zend_call_method); +static ZEND_FUNCTION(zend_object_init_with_constructor); static ZEND_FUNCTION(zend_test_zend_ini_parse_quantity); static ZEND_FUNCTION(zend_test_zend_ini_parse_uquantity); static ZEND_FUNCTION(zend_test_zend_ini_str); @@ -350,6 +356,7 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(zend_test_parameter_with_attribute, arginfo_zend_test_parameter_with_attribute) ZEND_FE(zend_get_current_func_name, arginfo_zend_get_current_func_name) ZEND_FE(zend_call_method, arginfo_zend_call_method) + ZEND_FE(zend_object_init_with_constructor, arginfo_zend_object_init_with_constructor) ZEND_FE(zend_test_zend_ini_parse_quantity, arginfo_zend_test_zend_ini_parse_quantity) ZEND_FE(zend_test_zend_ini_parse_uquantity, arginfo_zend_test_zend_ini_parse_uquantity) ZEND_FE(zend_test_zend_ini_str, arginfo_zend_test_zend_ini_str) diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt new file mode 100644 index 0000000000000..781ee05e12c37 --- /dev/null +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -0,0 +1,98 @@ +--TEST-- +Zend: Test zend_forbid_dynamic_call() for methods +--EXTENSIONS-- +zend_test +sysvmsg +--FILE-- +getMessage(), PHP_EOL; +} +try { + $o = zend_object_init_with_constructor("_ZendTestTrait"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $o = zend_object_init_with_constructor("ZendTestUnitEnum"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $o = zend_object_init_with_constructor("SysvMessageQueue"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $o = zend_object_init_with_constructor("PrivateUser"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $o = zend_object_init_with_constructor("ThrowingUser"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +echo "Testing param passing\n"; +try { + $o = zend_object_init_with_constructor("TestUser"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $o = zend_object_init_with_constructor("TestUser", "str", 5); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +try { + $o = zend_object_init_with_constructor("TestUser", 5, string_param: "str"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +Testing impossible initializations +Error: Cannot instantiate interface _ZendTestInterface +Error: Cannot instantiate trait _ZendTestTrait +Error: Cannot instantiate enum ZendTestUnitEnum +Error: Cannot directly construct SysvMessageQueue, use msg_get_queue() instead +Error: Call to private PrivateUser::__construct() from global scope +Exception: Don't construct +Testing param passing +ArgumentCountError: Too few arguments to function TestUser::__construct(), 0 passed and exactly 2 expected +TypeError: TestUser::__construct(): Argument #1 ($int_param) must be of type int, string given +object(TestUser)#3 (0) { +} From cebc0a24c7c07a114181e4348900c23811b2e5d5 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 01:22:23 +0100 Subject: [PATCH 04/14] nit --- Zend/zend_API.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 7d0111af09ee3..cc0495d361ded 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1852,7 +1852,7 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c if (UNEXPECTED(status == FAILURE)) { return FAILURE; } - zend_object * obj = Z_OBJ_P(arg); + zend_object *obj = Z_OBJ_P(arg); zend_function *constructor = obj->handlers->get_constructor(obj); if (UNEXPECTED(constructor == NULL)) { return FAILURE; From d50f1c36f5d93549b0e13b308b986395188789d6 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 16:31:18 +0100 Subject: [PATCH 05/14] Fix memory leak on case of failure --- Zend/zend_API.c | 5 +++++ ext/zend_test/test.c | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index cc0495d361ded..d75346ccf9a5c 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1850,11 +1850,14 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c { zend_result status = _object_and_properties_init(arg, class_type, NULL); if (UNEXPECTED(status == FAILURE)) { + ZVAL_UNDEF(arg); return FAILURE; } zend_object *obj = Z_OBJ_P(arg); zend_function *constructor = obj->handlers->get_constructor(obj); if (UNEXPECTED(constructor == NULL)) { + zval_ptr_dtor(arg); + ZVAL_UNDEF(arg); return FAILURE; } /* A constructor should not return a value, however if an exception is thrown @@ -1870,6 +1873,8 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c named_params ); if (Z_TYPE(retval) == IS_UNDEF) { + zval_ptr_dtor(arg); + ZVAL_UNDEF(arg); return FAILURE; } else { /* Unlikely, but user constructors may return any value they want */ diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 4632158e294fd..d63e89ed42868 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -473,11 +473,14 @@ static ZEND_FUNCTION(zend_object_init_with_constructor) Z_PARAM_VARIADIC_WITH_NAMED(args, num_args, named_args) ZEND_PARSE_PARAMETERS_END(); - zend_result status = object_init_with_constructor(return_value, ce, num_args, args, named_args); + zval obj; + /* We don't use return_value directly to check for memory leaks of the API on failure */ + zend_result status = object_init_with_constructor(&obj, ce, num_args, args, named_args); if (status == FAILURE) { RETURN_THROWS(); } ZEND_ASSERT(!EG(exception)); + ZVAL_COPY_VALUE(return_value, &obj); } static ZEND_FUNCTION(zend_get_unit_enum) From 5b2946d397670c55efc3655079c6bad8655258a6 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 16:31:49 +0100 Subject: [PATCH 06/14] Fix test description --- ext/zend_test/tests/zend_object_init_with_constructor.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt index 781ee05e12c37..7596c95117927 100644 --- a/ext/zend_test/tests/zend_object_init_with_constructor.phpt +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -1,5 +1,5 @@ --TEST-- -Zend: Test zend_forbid_dynamic_call() for methods +Zend: Test object_init_with_constructor() API --EXTENSIONS-- zend_test sysvmsg From 6d98dcec785f04a50baf053745c44067297cb821 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 16:34:05 +0100 Subject: [PATCH 07/14] Test abstract class initialization failure --- .../tests/zend_object_init_with_constructor.phpt | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt index 7596c95117927..a921e8d96ae70 100644 --- a/ext/zend_test/tests/zend_object_init_with_constructor.phpt +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -18,6 +18,12 @@ class ThrowingUser { } } +abstract class AbstractClass { + public function __construct() { + return new stdClass(); + } +} + class TestUser { public function __construct(int $int_param, string $string_param) { return new stdClass(); @@ -43,6 +49,12 @@ try { } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } +try { + $o = zend_object_init_with_constructor("AbstractClass"); + var_dump($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} try { $o = zend_object_init_with_constructor("SysvMessageQueue"); var_dump($o); @@ -88,11 +100,12 @@ Testing impossible initializations Error: Cannot instantiate interface _ZendTestInterface Error: Cannot instantiate trait _ZendTestTrait Error: Cannot instantiate enum ZendTestUnitEnum +Error: Cannot instantiate abstract class AbstractClass Error: Cannot directly construct SysvMessageQueue, use msg_get_queue() instead Error: Call to private PrivateUser::__construct() from global scope Exception: Don't construct Testing param passing ArgumentCountError: Too few arguments to function TestUser::__construct(), 0 passed and exactly 2 expected TypeError: TestUser::__construct(): Argument #1 ($int_param) must be of type int, string given -object(TestUser)#3 (0) { +object(TestUser)#1 (0) { } From 3f6e96e63f8391656bed1b671d530a52e3059db7 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 16:40:44 +0100 Subject: [PATCH 08/14] Add destructors --- .../zend_object_init_with_constructor.phpt | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt index a921e8d96ae70..1a054921a614f 100644 --- a/ext/zend_test/tests/zend_object_init_with_constructor.phpt +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -10,24 +10,36 @@ class PrivateUser { private function __construct() { return new stdClass(); } + public function __destruct() { + echo 'Destructor for ', __CLASS__, PHP_EOL; + } } class ThrowingUser { public function __construct() { throw new Exception("Don't construct"); } + public function __destruct() { + echo 'Destructor for ', __CLASS__, PHP_EOL; + } } abstract class AbstractClass { public function __construct() { return new stdClass(); } + public function __destruct() { + echo 'Destructor for ', __CLASS__, PHP_EOL; + } } class TestUser { public function __construct(int $int_param, string $string_param) { return new stdClass(); } + public function __destruct() { + echo 'Destructor for ', __CLASS__, PHP_EOL; + } } echo "Testing impossible initializations\n"; @@ -102,10 +114,15 @@ Error: Cannot instantiate trait _ZendTestTrait Error: Cannot instantiate enum ZendTestUnitEnum Error: Cannot instantiate abstract class AbstractClass Error: Cannot directly construct SysvMessageQueue, use msg_get_queue() instead +Destructor for PrivateUser Error: Call to private PrivateUser::__construct() from global scope +Destructor for ThrowingUser Exception: Don't construct Testing param passing +Destructor for TestUser ArgumentCountError: Too few arguments to function TestUser::__construct(), 0 passed and exactly 2 expected +Destructor for TestUser TypeError: TestUser::__construct(): Argument #1 ($int_param) must be of type int, string given -object(TestUser)#1 (0) { +object(TestUser)#3 (0) { } +Destructor for TestUser From bda84f4c172a5596bac4100b8b774414636050ab Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 16:52:43 +0100 Subject: [PATCH 09/14] Do not run destructors when an exception has been raised Found an existing bug with private constructors --- Zend/zend_API.c | 1 + ext/zend_test/tests/zend_object_init_with_constructor.phpt | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index d75346ccf9a5c..56815a9ffb051 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1873,6 +1873,7 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c named_params ); if (Z_TYPE(retval) == IS_UNDEF) { + zend_object_store_ctor_failed(obj); zval_ptr_dtor(arg); ZVAL_UNDEF(arg); return FAILURE; diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt index 1a054921a614f..f2fb9a3d98c99 100644 --- a/ext/zend_test/tests/zend_object_init_with_constructor.phpt +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -116,12 +116,9 @@ Error: Cannot instantiate abstract class AbstractClass Error: Cannot directly construct SysvMessageQueue, use msg_get_queue() instead Destructor for PrivateUser Error: Call to private PrivateUser::__construct() from global scope -Destructor for ThrowingUser Exception: Don't construct Testing param passing -Destructor for TestUser ArgumentCountError: Too few arguments to function TestUser::__construct(), 0 passed and exactly 2 expected -Destructor for TestUser TypeError: TestUser::__construct(): Argument #1 ($int_param) must be of type int, string given object(TestUser)#3 (0) { } From 37551517e61206dfae74f6977463b3810742b796 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Mon, 3 Jun 2024 17:24:31 +0100 Subject: [PATCH 10/14] Deal with NULL constructor properly --- Zend/zend_API.c | 19 ++++- .../zend_object_init_with_constructor.phpt | 83 +++++++++++++++++-- 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 56815a9ffb051..2354c98932650 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1855,10 +1855,21 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c } zend_object *obj = Z_OBJ_P(arg); zend_function *constructor = obj->handlers->get_constructor(obj); - if (UNEXPECTED(constructor == NULL)) { - zval_ptr_dtor(arg); - ZVAL_UNDEF(arg); - return FAILURE; + if (constructor == NULL) { + /* The constructor can be NULL for 2 different reasons: + * - It is not defined + * - We are not allowed to call the constructor (e.g. private, or internal opaque class) + * and an exception has been thrown + * in the former case, we are done and the object is initialized, + * in the latter we need to destroy the object as initialization failed + */ + if (UNEXPECTED(EG(exception))) { + zval_ptr_dtor(arg); + ZVAL_UNDEF(arg); + return FAILURE; + } else { + return SUCCESS; + } } /* A constructor should not return a value, however if an exception is thrown * zend_call_known_function() will set the retval to IS_UNDEF */ diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt index f2fb9a3d98c99..7dbf1efbb6beb 100644 --- a/ext/zend_test/tests/zend_object_init_with_constructor.phpt +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -33,7 +33,7 @@ abstract class AbstractClass { } } -class TestUser { +class TestUserWithConstructorArgs { public function __construct(int $int_param, string $string_param) { return new stdClass(); } @@ -42,70 +42,121 @@ class TestUser { } } +class TestUserWithConstructorNoParams { + public function __construct() { + return new stdClass(); + } + public function __destruct() { + echo 'Destructor for ', __CLASS__, PHP_EOL; + } +} + +class TestUserWithoutConstructor { + public function __destruct() { + echo 'Destructor for ', __CLASS__, PHP_EOL; + } +} + echo "Testing impossible initializations\n"; try { $o = zend_object_init_with_constructor("_ZendTestInterface"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { $o = zend_object_init_with_constructor("_ZendTestTrait"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { $o = zend_object_init_with_constructor("ZendTestUnitEnum"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { $o = zend_object_init_with_constructor("AbstractClass"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { $o = zend_object_init_with_constructor("SysvMessageQueue"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { $o = zend_object_init_with_constructor("PrivateUser"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { $o = zend_object_init_with_constructor("ThrowingUser"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } echo "Testing param passing\n"; try { - $o = zend_object_init_with_constructor("TestUser"); + $o = zend_object_init_with_constructor("TestUserWithConstructorArgs"); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { - $o = zend_object_init_with_constructor("TestUser", "str", 5); + $o = zend_object_init_with_constructor("TestUserWithConstructorArgs", "str", 5); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } try { - $o = zend_object_init_with_constructor("TestUser", 5, string_param: "str"); + $o = zend_object_init_with_constructor("TestUserWithConstructorArgs", 5, string_param: "str", unused_param: 15.3); var_dump($o); + unset($o); } catch (\Throwable $e) { echo $e::class, ': ', $e->getMessage(), PHP_EOL; } +$o = zend_object_init_with_constructor("TestUserWithConstructorArgs", 5, string_param: "str"); +var_dump($o); +unset($o); + +echo "Passing too many args to constructor\n"; +$o = zend_object_init_with_constructor("TestUserWithConstructorArgs", 5, "str", 'unused_param'); +var_dump($o); +unset($o); + +echo "Testing class with defined constructor and no params\n"; +$o = zend_object_init_with_constructor("TestUserWithConstructorNoParams"); +var_dump($o); +unset($o); + +echo "Testing class without defined constructor\n"; +try { + $o = zend_object_init_with_constructor("TestUserWithoutConstructor", 5, string_param: "str"); + var_dump($o); + unset($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +$o = zend_object_init_with_constructor("TestUserWithoutConstructor"); +var_dump($o); +unset($o); + ?> --EXPECT-- Testing impossible initializations @@ -118,8 +169,24 @@ Destructor for PrivateUser Error: Call to private PrivateUser::__construct() from global scope Exception: Don't construct Testing param passing -ArgumentCountError: Too few arguments to function TestUser::__construct(), 0 passed and exactly 2 expected -TypeError: TestUser::__construct(): Argument #1 ($int_param) must be of type int, string given -object(TestUser)#3 (0) { +ArgumentCountError: Too few arguments to function TestUserWithConstructorArgs::__construct(), 0 passed and exactly 2 expected +TypeError: TestUserWithConstructorArgs::__construct(): Argument #1 ($int_param) must be of type int, string given +Error: Unknown named parameter $unused_param +object(TestUserWithConstructorArgs)#1 (0) { +} +Destructor for TestUserWithConstructorArgs +Passing too many args to constructor +object(TestUserWithConstructorArgs)#1 (0) { +} +Destructor for TestUserWithConstructorArgs +Testing class with defined constructor and no params +object(TestUserWithConstructorNoParams)#1 (0) { +} +Destructor for TestUserWithConstructorNoParams +Testing class without defined constructor +object(TestUserWithoutConstructor)#1 (0) { +} +Destructor for TestUserWithoutConstructor +object(TestUserWithoutConstructor)#1 (0) { } -Destructor for TestUser +Destructor for TestUserWithoutConstructor From 94724be33a26e2d7e7a72f96752e67710a6b468c Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 4 Jun 2024 01:38:47 +0100 Subject: [PATCH 11/14] Fix behaviour with new for internal and userland classes Turns out the logic is really baffling --- Zend/zend_API.c | 21 ++- .../zend_object_init_with_constructor.phpt | 26 --- ...nstructor_classes_without_constructor.phpt | 149 ++++++++++++++++++ 3 files changed, 168 insertions(+), 28 deletions(-) create mode 100644 ext/zend_test/tests/zend_object_init_with_constructor_classes_without_constructor.phpt diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 2354c98932650..7be9c6bac4061 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1860,7 +1860,7 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c * - It is not defined * - We are not allowed to call the constructor (e.g. private, or internal opaque class) * and an exception has been thrown - * in the former case, we are done and the object is initialized, + * in the former case, we are (mostly) done and the object is initialized, * in the latter we need to destroy the object as initialization failed */ if (UNEXPECTED(EG(exception))) { @@ -1868,7 +1868,23 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c ZVAL_UNDEF(arg); return FAILURE; } else { - return SUCCESS; + /* Surprisingly, this is the only case where internal classes will allow to pass extra arguments + * However, if there are named arguments, an Error must be thrown to be consistent with new ClassName() */ + if (UNEXPECTED(named_params != NULL)) { + /* Throw standard Error */ + zend_string *arg_name = NULL; + zend_hash_get_current_key(named_params, &arg_name, /* num_index */ NULL); + ZEND_ASSERT(arg_name != NULL); + zend_throw_error(NULL, "Unknown named parameter $%s", ZSTR_VAL(arg_name)); + zend_string_release(arg_name); + /* Do not call destructor, free object, and set arg to IS_UNDEF */ + zend_object_store_ctor_failed(obj); + zval_ptr_dtor(arg); + ZVAL_UNDEF(arg); + return FAILURE; + } else { + return SUCCESS; + } } } /* A constructor should not return a value, however if an exception is thrown @@ -1884,6 +1900,7 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c named_params ); if (Z_TYPE(retval) == IS_UNDEF) { + /* Do not call destructor, free object, and set arg to IS_UNDEF */ zend_object_store_ctor_failed(obj); zval_ptr_dtor(arg); ZVAL_UNDEF(arg); diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt index 7dbf1efbb6beb..70dd23bdb2569 100644 --- a/ext/zend_test/tests/zend_object_init_with_constructor.phpt +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -51,12 +51,6 @@ class TestUserWithConstructorNoParams { } } -class TestUserWithoutConstructor { - public function __destruct() { - echo 'Destructor for ', __CLASS__, PHP_EOL; - } -} - echo "Testing impossible initializations\n"; try { $o = zend_object_init_with_constructor("_ZendTestInterface"); @@ -144,19 +138,6 @@ echo "Testing class with defined constructor and no params\n"; $o = zend_object_init_with_constructor("TestUserWithConstructorNoParams"); var_dump($o); unset($o); - -echo "Testing class without defined constructor\n"; -try { - $o = zend_object_init_with_constructor("TestUserWithoutConstructor", 5, string_param: "str"); - var_dump($o); - unset($o); -} catch (\Throwable $e) { - echo $e::class, ': ', $e->getMessage(), PHP_EOL; -} -$o = zend_object_init_with_constructor("TestUserWithoutConstructor"); -var_dump($o); -unset($o); - ?> --EXPECT-- Testing impossible initializations @@ -183,10 +164,3 @@ Testing class with defined constructor and no params object(TestUserWithConstructorNoParams)#1 (0) { } Destructor for TestUserWithConstructorNoParams -Testing class without defined constructor -object(TestUserWithoutConstructor)#1 (0) { -} -Destructor for TestUserWithoutConstructor -object(TestUserWithoutConstructor)#1 (0) { -} -Destructor for TestUserWithoutConstructor diff --git a/ext/zend_test/tests/zend_object_init_with_constructor_classes_without_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor_classes_without_constructor.phpt new file mode 100644 index 0000000000000..1adaaafaa195b --- /dev/null +++ b/ext/zend_test/tests/zend_object_init_with_constructor_classes_without_constructor.phpt @@ -0,0 +1,149 @@ +--TEST-- +Zend: Test object_init_with_constructor() API for objects without constructors +--EXTENSIONS-- +zend_test +--FILE-- +getMessage(), PHP_EOL; +} +echo "Using zend_object_init_with_constructor():\n"; +try { + $o = zend_object_init_with_constructor("_ZendTestMagicCall", 'position_arg'); + var_dump($o); + unset($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +echo "\n#### Passing extra named args ####\n"; +echo "Userland class:\n"; +echo "Using new:\n"; +try { + $o = new TestUserWithoutConstructor(unknown_param: 'named_arg'); + var_dump($o); + unset($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +echo "Using zend_object_init_with_constructor():\n"; +try { + $o = zend_object_init_with_constructor("TestUserWithoutConstructor", unknown_param: 'named_arg'); + var_dump($o); + unset($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +echo "Internal class:\n"; +echo "Using new:\n"; +try { + $o = new _ZendTestMagicCall(unknown_param: 'named_arg'); + var_dump($o); + unset($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +echo "Using zend_object_init_with_constructor():\n"; +try { + $o = zend_object_init_with_constructor("_ZendTestMagicCall", unknown_param: 'named_arg'); + var_dump($o); + unset($o); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +#### Passing no args #### +Userland class: +Using new: +object(TestUserWithoutConstructor)#1 (0) { +} +Destructor for TestUserWithoutConstructor +Using zend_object_init_with_constructor(): +object(TestUserWithoutConstructor)#1 (0) { +} +Destructor for TestUserWithoutConstructor +Internal class: +Using new: +object(_ZendTestMagicCall)#1 (0) { +} +Using zend_object_init_with_constructor(): +object(_ZendTestMagicCall)#1 (0) { +} + +#### Passing extra positional args #### +Userland class: +Using new: +object(TestUserWithoutConstructor)#1 (0) { +} +Destructor for TestUserWithoutConstructor +Using zend_object_init_with_constructor(): +object(TestUserWithoutConstructor)#1 (0) { +} +Destructor for TestUserWithoutConstructor +Internal class: +Using new: +object(_ZendTestMagicCall)#1 (0) { +} +Using zend_object_init_with_constructor(): +object(_ZendTestMagicCall)#1 (0) { +} + +#### Passing extra named args #### +Userland class: +Using new: +Error: Unknown named parameter $unknown_param +Using zend_object_init_with_constructor(): +Error: Unknown named parameter $unknown_param +Internal class: +Using new: +Error: Unknown named parameter $unknown_param +Using zend_object_init_with_constructor(): +Error: Unknown named parameter $unknown_param From 0f2a38e1168b763d495236f3ad6028794a04df13 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 6 Jun 2024 15:57:31 +0100 Subject: [PATCH 12/14] Fix test after rebase --- ext/zend_test/tests/zend_object_init_with_constructor.phpt | 1 - 1 file changed, 1 deletion(-) diff --git a/ext/zend_test/tests/zend_object_init_with_constructor.phpt b/ext/zend_test/tests/zend_object_init_with_constructor.phpt index 70dd23bdb2569..65b111447f0bf 100644 --- a/ext/zend_test/tests/zend_object_init_with_constructor.phpt +++ b/ext/zend_test/tests/zend_object_init_with_constructor.phpt @@ -146,7 +146,6 @@ Error: Cannot instantiate trait _ZendTestTrait Error: Cannot instantiate enum ZendTestUnitEnum Error: Cannot instantiate abstract class AbstractClass Error: Cannot directly construct SysvMessageQueue, use msg_get_queue() instead -Destructor for PrivateUser Error: Call to private PrivateUser::__construct() from global scope Exception: Don't construct Testing param passing From 610ca7365126ac949067e4ed611b8888043eca53 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 6 Jun 2024 16:03:06 +0100 Subject: [PATCH 13/14] Fix review nits --- Zend/zend_API.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 7be9c6bac4061..e02e0031970e4 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1867,24 +1867,24 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c zval_ptr_dtor(arg); ZVAL_UNDEF(arg); return FAILURE; + } + + /* Surprisingly, this is the only case where internal classes will allow to pass extra arguments + * However, if there are named arguments, an Error must be thrown to be consistent with new ClassName() */ + if (UNEXPECTED(named_params != NULL)) { + /* Throw standard Error */ + zend_string *arg_name = NULL; + zend_hash_get_current_key(named_params, &arg_name, /* num_index */ NULL); + ZEND_ASSERT(arg_name != NULL); + zend_throw_error(NULL, "Unknown named parameter $%s", ZSTR_VAL(arg_name)); + zend_string_release(arg_name); + /* Do not call destructor, free object, and set arg to IS_UNDEF */ + zend_object_store_ctor_failed(obj); + zval_ptr_dtor(arg); + ZVAL_UNDEF(arg); + return FAILURE; } else { - /* Surprisingly, this is the only case where internal classes will allow to pass extra arguments - * However, if there are named arguments, an Error must be thrown to be consistent with new ClassName() */ - if (UNEXPECTED(named_params != NULL)) { - /* Throw standard Error */ - zend_string *arg_name = NULL; - zend_hash_get_current_key(named_params, &arg_name, /* num_index */ NULL); - ZEND_ASSERT(arg_name != NULL); - zend_throw_error(NULL, "Unknown named parameter $%s", ZSTR_VAL(arg_name)); - zend_string_release(arg_name); - /* Do not call destructor, free object, and set arg to IS_UNDEF */ - zend_object_store_ctor_failed(obj); - zval_ptr_dtor(arg); - ZVAL_UNDEF(arg); - return FAILURE; - } else { - return SUCCESS; - } + return SUCCESS; } } /* A constructor should not return a value, however if an exception is thrown @@ -1894,7 +1894,7 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c constructor, obj, class_type, - /* retval */ &retval, + &retval, param_count, params, named_params From 9ad870a526341cdf7ab3a6b8466e1334ba2e59b0 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Thu, 6 Jun 2024 16:13:36 +0100 Subject: [PATCH 14/14] Allow empty HashTable --- Zend/zend_API.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index e02e0031970e4..bc6401c8ca28e 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -1870,8 +1870,9 @@ ZEND_API zend_result object_init_with_constructor(zval *arg, zend_class_entry *c } /* Surprisingly, this is the only case where internal classes will allow to pass extra arguments - * However, if there are named arguments, an Error must be thrown to be consistent with new ClassName() */ - if (UNEXPECTED(named_params != NULL)) { + * However, if there are named arguments (and it is not empty), + * an Error must be thrown to be consistent with new ClassName() */ + if (UNEXPECTED(named_params != NULL && zend_hash_num_elements(named_params) != 0)) { /* Throw standard Error */ zend_string *arg_name = NULL; zend_hash_get_current_key(named_params, &arg_name, /* num_index */ NULL);