Skip to content

Commit f36150c

Browse files
nicolas-grekasnicolas-grekas
committed
Fix reassigning when calling the constructor twice
1 parent fdb5068 commit f36150c

File tree

8 files changed

+722
-11
lines changed

8 files changed

+722
-11
lines changed

‎Zend/tests/readonly_props/cpp_reassign_direct_ctor_call.phpt‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
Promoted readonly properties cannot be reassigned when __construct() is called directly
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly string $value = 'default',
9+
) {
10+
// Don't use reassignment here, leave it available
11+
}
12+
}
13+
14+
$obj = new Foo('initial');
15+
echo "Initial value: " . $obj->value . "\n";
16+
17+
// Direct call to __construct() should NOT allow reassignment
18+
try {
19+
$obj->__construct('modified');
20+
echo "After direct __construct: " . $obj->value . "\n";
21+
} catch (Error $e) {
22+
echo "Error: " . $e->getMessage() . "\n";
23+
}
24+
25+
// Also test with a class that uses reassignment
26+
class Bar {
27+
public function __construct(
28+
public readonly string $value = 'default',
29+
) {
30+
$this->value = strtoupper($this->value);
31+
}
32+
}
33+
34+
$bar = new Bar('hello');
35+
echo "Bar initial value: " . $bar->value . "\n";
36+
37+
// Direct call should fail during the CPP assignment (property not UNINIT)
38+
// Note: The error happens inside the constructor because CPP assignment happens first
39+
try {
40+
$bar->__construct('world');
41+
echo "After direct __construct: " . $bar->value . "\n";
42+
} catch (Error $e) {
43+
echo "Error: " . $e->getMessage() . "\n";
44+
}
45+
46+
?>
47+
--EXPECT--
48+
Initial value: initial
49+
Error: Cannot modify readonly property Foo::$value
50+
Bar initial value: HELLO
51+
Error: Cannot modify readonly property Bar::$value

‎Zend/tests/readonly_props/cpp_reassign_reflection.phpt‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
--TEST--
2+
Promoted readonly property reassignment works when object created via reflection
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly string $bar = 'default',
9+
) {
10+
$this->bar = 'overwritten in constructor';
11+
}
12+
}
13+
14+
// Test 1: Object created via reflection without constructor, then __construct() called
15+
echo "Test 1: Reflection newInstanceWithoutConstructor + explicit __construct()\n";
16+
$ref = new ReflectionClass(Foo::class);
17+
$obj = $ref->newInstanceWithoutConstructor();
18+
19+
// Property should be uninitialized at this point
20+
try {
21+
echo $obj->bar;
22+
echo "ERROR: Should have thrown for uninitialized property\n";
23+
} catch (Error $e) {
24+
echo "OK: " . $e->getMessage() . "\n";
25+
}
26+
27+
// Now call constructor - reassignment should work
28+
$obj->__construct('explicit call');
29+
echo "After __construct: " . $obj->bar . "\n";
30+
31+
// Second __construct() call should fail
32+
echo "\nTest 2: Second __construct() call should fail\n";
33+
try {
34+
$obj->__construct('second call');
35+
echo "ERROR: Second __construct() should have failed\n";
36+
} catch (Error $e) {
37+
echo "OK: " . $e->getMessage() . "\n";
38+
}
39+
40+
// Test 3: Normal new still works
41+
echo "\nTest 3: Normal 'new' still works\n";
42+
$obj2 = new Foo('via new');
43+
echo "After new: " . $obj2->bar . "\n";
44+
45+
// Second call should fail
46+
try {
47+
$obj2->__construct('second call');
48+
echo "ERROR: Should have failed\n";
49+
} catch (Error $e) {
50+
echo "OK: " . $e->getMessage() . "\n";
51+
}
52+
53+
// Test 4: Reflection newInstanceArgs (calls constructor)
54+
echo "\nTest 4: Reflection newInstanceArgs\n";
55+
$obj3 = $ref->newInstanceArgs(['via newInstanceArgs']);
56+
echo "After newInstanceArgs: " . $obj3->bar . "\n";
57+
58+
// Second call should fail
59+
try {
60+
$obj3->__construct('second call');
61+
echo "ERROR: Should have failed\n";
62+
} catch (Error $e) {
63+
echo "OK: " . $e->getMessage() . "\n";
64+
}
65+
66+
?>
67+
--EXPECT--
68+
Test 1: Reflection newInstanceWithoutConstructor + explicit __construct()
69+
OK: Typed property Foo::$bar must not be accessed before initialization
70+
After __construct: overwritten in constructor
71+
72+
Test 2: Second __construct() call should fail
73+
OK: Cannot modify readonly property Foo::$bar
74+
75+
Test 3: Normal 'new' still works
76+
After new: overwritten in constructor
77+
OK: Cannot modify readonly property Foo::$bar
78+
79+
Test 4: Reflection newInstanceArgs
80+
After newInstanceArgs: overwritten in constructor
81+
OK: Cannot modify readonly property Foo::$bar

‎Zend/zend_object_handlers.c‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
/* Check if we're within a constructor call chain for the given object.
5050
* Walks up the call stack to find if any frame is a constructor for zobj.
5151
* This allows reassignment from the constructor or methods/closures called from it. */
52-
ZEND_API bool zend_is_in_constructor(const zend_object *zobj)
52+
static bool zend_is_in_constructor(const zend_object *zobj)
5353
{
5454
zend_execute_data *ex = EG(current_execute_data);
5555
while (ex) {
@@ -64,6 +64,20 @@ ZEND_API bool zend_is_in_constructor(const zend_object *zobj)
6464
return false;
6565
}
6666

67+
/* Check if we're in the FIRST construction of an object.
68+
* Uses the IS_OBJ_CTOR_CALLED flag which is set when a constructor completes.
69+
* This allows both 'new' and explicit __construct() calls on reflection-created objects. */
70+
ZEND_API bool zend_is_in_original_construction(const zend_object *zobj)
71+
{
72+
/* If a constructor has already completed on this object, this is not original construction */
73+
if (zobj->extra_flags & IS_OBJ_CTOR_CALLED) {
74+
return false;
75+
}
76+
77+
/* Verify we're actually in a constructor for this object */
78+
return zend_is_in_constructor(zobj);
79+
}
80+
6781
static zend_arg_info zend_call_trampoline_arginfo[1] = {{0}};
6882
static zend_arg_info zend_property_hook_arginfo[1] = {{0}};
6983

‎Zend/zend_object_handlers.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ ZEND_API zend_result zend_std_get_closure(zend_object *obj, zend_class_entry **c
276276
ZEND_API HashTable *rebuild_object_properties_internal(zend_object *zobj);
277277
ZEND_API ZEND_COLD zend_never_inline void zend_bad_method_call(const zend_function *fbc, const zend_string *method_name, const zend_class_entry *scope);
278278
ZEND_API ZEND_COLD zend_never_inline void zend_abstract_method_call(const zend_function *fbc);
279-
ZEND_API bool zend_is_in_constructor(const zend_object *zobj);
279+
ZEND_API bool zend_is_in_original_construction(const zend_object *zobj);
280280

281281
/* Check if a readonly property can be modified (has REINITABLE or CPP_REINITABLE flag and is in constructor) */
282282
static zend_always_inline bool zend_is_readonly_property_modifiable(zval *property_val, const zend_property_info *prop_info, zend_object *zobj)
@@ -285,7 +285,7 @@ static zend_always_inline bool zend_is_readonly_property_modifiable(zval *proper
285285
return true;
286286
}
287287
if ((Z_PROP_FLAG_P(property_val) & IS_PROP_CPP_REINITABLE)
288-
&& zend_is_in_constructor(zobj)) {
288+
&& zend_is_in_original_construction(zobj)) {
289289
return true;
290290
}
291291
return false;

‎Zend/zend_types.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,7 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
858858

859859
#define IS_OBJ_LAZY_UNINITIALIZED (1U<<31) /* Virtual proxy or uninitialized Ghost */
860860
#define IS_OBJ_LAZY_PROXY (1U<<30) /* Virtual proxy (may be initialized) */
861+
#define IS_OBJ_CTOR_CALLED (1U<<29) /* A constructor has completed on this object */
861862

862863
#define OBJ_EXTRA_FLAGS(obj) ((obj)->extra_flags)
863864

‎Zend/zend_vm_def.h‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2975,7 +2975,12 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
29752975
call_info = EX_CALL_INFO();
29762976
#endif
29772977
if (UNEXPECTED(call_info & ZEND_CALL_RELEASE_THIS)) {
2978-
OBJ_RELEASE(Z_OBJ(execute_data->This));
2978+
zend_object *obj = Z_OBJ(execute_data->This);
2979+
/* Mark that a constructor has completed on this object */
2980+
if (EX(func)->common.fn_flags & ZEND_ACC_CTOR) {
2981+
obj->extra_flags |= IS_OBJ_CTOR_CALLED;
2982+
}
2983+
OBJ_RELEASE(obj);
29792984
} else if (UNEXPECTED(call_info & ZEND_CALL_CLOSURE)) {
29802985
OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func)));
29812986
}
@@ -3009,7 +3014,12 @@ ZEND_VM_HOT_HELPER(zend_leave_helper, ANY, ANY)
30093014
zend_vm_stack_free_extra_args_ex(call_info, execute_data);
30103015

30113016
if (UNEXPECTED(call_info & ZEND_CALL_RELEASE_THIS)) {
3012-
OBJ_RELEASE(Z_OBJ(execute_data->This));
3017+
zend_object *obj = Z_OBJ(execute_data->This);
3018+
/* Mark that a constructor has completed on this object */
3019+
if (EX(func)->common.fn_flags & ZEND_ACC_CTOR) {
3020+
obj->extra_flags |= IS_OBJ_CTOR_CALLED;
3021+
}
3022+
OBJ_RELEASE(obj);
30133023
} else if (UNEXPECTED(call_info & ZEND_CALL_CLOSURE)) {
30143024
OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func)));
30153025
}

‎Zend/zend_vm_execute.h‎

Lines changed: 36 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)