Skip to content

Commit 3bcf2c3

Browse files
authoredFeb 28, 2023
Allow readonly properties to be reinitialized once during cloning (#10389)
RFC: https://wiki.php.net/rfc/readonly_amendments
1 parent b1ccbc8 commit 3bcf2c3

17 files changed

+385
-27
lines changed
 
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
Readonly property cannot be reset twice during cloning
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly int $bar
9+
) {}
10+
11+
public function __clone()
12+
{
13+
$this->bar = 2;
14+
var_dump($this);
15+
$this->bar = 3;
16+
}
17+
}
18+
19+
$foo = new Foo(1);
20+
21+
try {
22+
clone $foo;
23+
} catch (Error $exception) {
24+
echo $exception->getMessage() . "\n";
25+
}
26+
27+
echo "done";
28+
29+
?>
30+
--EXPECT--
31+
object(Foo)#2 (1) {
32+
["bar"]=>
33+
int(2)
34+
}
35+
Cannot modify readonly property Foo::$bar
36+
done
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
Readonly property cannot be reset after cloning when there is no custom clone handler
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly int $bar,
9+
public readonly int $baz
10+
) {}
11+
12+
public function wrongCloneOld()
13+
{
14+
$instance = clone $this;
15+
$this->bar++;
16+
}
17+
18+
public function wrongCloneNew()
19+
{
20+
$instance = clone $this;
21+
$instance->baz++;
22+
}
23+
}
24+
25+
$foo = new Foo(1, 1);
26+
27+
try {
28+
$foo->wrongCloneOld();
29+
} catch (Error $exception) {
30+
echo $exception->getMessage() . "\n";
31+
}
32+
33+
try {
34+
$foo->wrongCloneNew();
35+
} catch (Error $exception) {
36+
echo $exception->getMessage() . "\n";
37+
}
38+
39+
?>
40+
--EXPECT--
41+
Cannot modify readonly property Foo::$bar
42+
Cannot modify readonly property Foo::$baz
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
Readonly property cannot be reset after cloning when there is a custom clone handler
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly int $bar,
9+
public readonly int $baz
10+
) {}
11+
12+
public function __clone() {}
13+
14+
public function wrongCloneOld()
15+
{
16+
$instance = clone $this;
17+
$this->bar++;
18+
}
19+
20+
public function wrongCloneNew()
21+
{
22+
$instance = clone $this;
23+
$instance->baz++;
24+
}
25+
}
26+
27+
$foo = new Foo(1, 1);
28+
29+
try {
30+
$foo->wrongCloneOld();
31+
} catch (Error $exception) {
32+
echo $exception->getMessage() . "\n";
33+
}
34+
35+
try {
36+
$foo->wrongCloneNew();
37+
} catch (Error $exception) {
38+
echo $exception->getMessage() . "\n";
39+
}
40+
41+
?>
42+
--EXPECT--
43+
Cannot modify readonly property Foo::$bar
44+
Cannot modify readonly property Foo::$baz
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
Readonly property can be reset once during cloning
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly int $bar
9+
) {}
10+
11+
public function __clone()
12+
{
13+
$this->bar++;
14+
}
15+
}
16+
17+
$foo = new Foo(1);
18+
19+
var_dump(clone $foo);
20+
21+
$foo2 = clone $foo;
22+
var_dump($foo2);
23+
24+
var_dump(clone $foo2);
25+
26+
?>
27+
--EXPECTF--
28+
object(Foo)#%d (%d) {
29+
["bar"]=>
30+
int(2)
31+
}
32+
object(Foo)#%d (%d) {
33+
["bar"]=>
34+
int(2)
35+
}
36+
object(Foo)#%d (%d) {
37+
["bar"]=>
38+
int(3)
39+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
Test that __clone() unset and reassign properties
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly stdClass $bar,
9+
) {}
10+
11+
public function __clone()
12+
{
13+
unset($this->bar);
14+
var_dump($this);
15+
$this->bar = new stdClass();
16+
}
17+
}
18+
19+
$foo = new Foo(new stdClass());
20+
var_dump($foo);
21+
$foo2 = clone $foo;
22+
23+
var_dump($foo);
24+
var_dump($foo2);
25+
26+
?>
27+
--EXPECTF--
28+
object(Foo)#1 (%d) {
29+
["bar"]=>
30+
object(stdClass)#2 (%d) {
31+
}
32+
}
33+
object(Foo)#3 (%d) {
34+
["bar"]=>
35+
uninitialized(stdClass)
36+
}
37+
object(Foo)#1 (%d) {
38+
["bar"]=>
39+
object(stdClass)#2 (%d) {
40+
}
41+
}
42+
object(Foo)#3 (%d) {
43+
["bar"]=>
44+
object(stdClass)#4 (%d) {
45+
}
46+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
__clone() can indirectly modify unlocked readonly properties
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly array $bar
9+
) {}
10+
11+
public function __clone()
12+
{
13+
$this->bar['bar'] = 'bar';
14+
}
15+
}
16+
17+
$foo = new Foo([]);
18+
// First call fills the cache slot
19+
var_dump(clone $foo);
20+
var_dump(clone $foo);
21+
22+
?>
23+
--EXPECTF--
24+
object(Foo)#2 (%d) {
25+
["bar"]=>
26+
array(1) {
27+
["bar"]=>
28+
string(3) "bar"
29+
}
30+
}
31+
object(Foo)#2 (%d) {
32+
["bar"]=>
33+
array(1) {
34+
["bar"]=>
35+
string(3) "bar"
36+
}
37+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
Readonly property can be reset once during cloning even after a type error
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public function __construct(
8+
public readonly int $bar
9+
) {}
10+
11+
public function __clone()
12+
{
13+
try {
14+
$this->bar = "foo";
15+
} catch (Error $e) {
16+
echo $e->getMessage() . "\n";
17+
}
18+
19+
$this->bar = 2;
20+
}
21+
}
22+
23+
$foo = new Foo(1);
24+
25+
var_dump(clone $foo);
26+
var_dump(clone $foo);
27+
28+
?>
29+
--EXPECTF--
30+
Cannot assign string to property Foo::$bar of type int
31+
object(Foo)#%d (%d) {
32+
["bar"]=>
33+
int(2)
34+
}
35+
Cannot assign string to property Foo::$bar of type int
36+
object(Foo)#%d (%d) {
37+
["bar"]=>
38+
int(2)
39+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
Test failing readonly assignment with coercion
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
public readonly string $bar;
8+
9+
public function __construct() {
10+
$this->bar = 'bar';
11+
try {
12+
$this->bar = 42;
13+
} catch (Error $e) {
14+
echo $e->getMessage(), "\n";
15+
}
16+
}
17+
}
18+
19+
new Foo();
20+
21+
?>
22+
--EXPECTF--
23+
Cannot modify readonly property Foo::$bar

‎Zend/zend_API.c

+4
Original file line numberDiff line numberDiff line change
@@ -4224,6 +4224,10 @@ ZEND_API zend_property_info *zend_declare_typed_property(zend_class_entry *ce, z
42244224
ce->ce_flags |= ZEND_ACC_HAS_TYPE_HINTS;
42254225
}
42264226

4227+
if (access_type & ZEND_ACC_READONLY) {
4228+
ce->ce_flags |= ZEND_ACC_HAS_READONLY_PROPS;
4229+
}
4230+
42274231
if (ce->type == ZEND_INTERNAL_CLASS) {
42284232
property_info = pemalloc(sizeof(zend_property_info), 1);
42294233
} else {

‎Zend/zend_compile.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ typedef struct _zend_oparray_context {
241241
/* or IS_CONSTANT_VISITED_MARK | | | */
242242
#define ZEND_CLASS_CONST_IS_CASE (1 << 6) /* | | | X */
243243
/* | | | */
244-
/* Class Flags (unused: 21,30,31) | | | */
244+
/* Class Flags (unused: 30,31) | | | */
245245
/* =========== | | | */
246246
/* | | | */
247247
/* Special class types | | | */
@@ -288,6 +288,8 @@ typedef struct _zend_oparray_context {
288288
/* | | | */
289289
/* Class is linked apart from variance obligations. | | | */
290290
#define ZEND_ACC_NEARLY_LINKED (1 << 20) /* X | | | */
291+
/* Class has readonly props | | | */
292+
#define ZEND_ACC_HAS_READONLY_PROPS (1 << 21) /* X | | | */
291293
/* | | | */
292294
/* stored in opcache (may be partially) | | | */
293295
#define ZEND_ACC_CACHED (1 << 22) /* X | | | */

‎Zend/zend_enum.c

+11-3
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,17 @@ zend_object *zend_enum_new(zval *result, zend_class_entry *ce, zend_string *case
4141
zend_object *zobj = zend_objects_new(ce);
4242
ZVAL_OBJ(result, zobj);
4343

44-
ZVAL_STR_COPY(OBJ_PROP_NUM(zobj, 0), case_name);
44+
zval *zname = OBJ_PROP_NUM(zobj, 0);
45+
ZVAL_STR_COPY(zname, case_name);
46+
/* ZVAL_COPY does not set Z_PROP_FLAG, this needs to be cleared to avoid leaving IS_PROP_REINITABLE set */
47+
Z_PROP_FLAG_P(zname) = 0;
48+
4549
if (backing_value_zv != NULL) {
46-
ZVAL_COPY(OBJ_PROP_NUM(zobj, 1), backing_value_zv);
50+
zval *prop = OBJ_PROP_NUM(zobj, 1);
51+
52+
ZVAL_COPY(prop, backing_value_zv);
53+
/* ZVAL_COPY does not set Z_PROP_FLAG, this needs to be cleared to avoid leaving IS_PROP_REINITABLE set */
54+
Z_PROP_FLAG_P(prop) = 0;
4755
}
4856

4957
return zobj;
@@ -179,7 +187,7 @@ void zend_enum_add_interfaces(zend_class_entry *ce)
179187

180188
if (ce->enum_backing_type != IS_UNDEF) {
181189
ce->interface_names[num_interfaces_before + 1].name = zend_string_copy(zend_ce_backed_enum->name);
182-
ce->interface_names[num_interfaces_before + 1].lc_name = zend_string_init("backedenum", sizeof("backedenum") - 1, 0);
190+
ce->interface_names[num_interfaces_before + 1].lc_name = zend_string_init("backedenum", sizeof("backedenum") - 1, 0);
183191
}
184192

185193
ce->default_object_handlers = &zend_enum_object_handlers;

0 commit comments

Comments
 (0)
Please sign in to comment.