Deal with NULL constructor properly

Author: Girgias

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 */

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