Skip to content

modify readonly class property with reference #10844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
varrg opened this issue Mar 13, 2023 · 3 comments
Closed

modify readonly class property with reference #10844

varrg opened this issue Mar 13, 2023 · 3 comments

Comments

@varrg
Copy link

varrg commented Mar 13, 2023

Description

The following code:

<?php
readonly class A implements IteratorAggregate {
	function __construct(readonly public string $foo = 'bar') {}
	
	function getIterator() : Traversable {
		return new ArrayIterator($this);
	}
}

$obj = new A;
//$obj->foo = 'baz';  // expected fatal error
//$prop = &$obj->foo;  // expected fatal error
foreach ($obj as $k => &$v) {
	$v = 'baz';
}

var_dump($obj);

Resulted in this output:

object(A)#1 (1) {
  ["foo"]=>
  &string(3) "baz"
}

But I expected this output instead:

Fatal error: Uncaught Error: Cannot modify readonly property A::$foo

PHP Version

8.2.3

Operating System

No response

@nielsdos
Copy link
Member

Looks very related to #9432

@iluuu1994
Copy link
Member

iluuu1994 commented Mar 17, 2023

Also related, it's possible to skip the reference type check:

class A implements IteratorAggregate {
    function __construct(
        public string $foo = 'bar'
    ) {}

    function getIterator(): Traversable {
        return new ArrayIterator($this);
    }
}

$obj = new A;
foreach ($obj as $k => &$v) {
    $v = 42;
}

var_dump($obj);
// object(A)#1 (1) {
//   ["foo"]=>
//   &int(42)
// }

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 17, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 17, 2023
@iluuu1994
Copy link
Member

This seems fairly difficult to fix. The problem is that ArrayIterator copies the properties array without knowing whether the operation is R or RW and ZEND_FE_FETCH_RW converts the given value to a reference without knowing that the value came from an object. Thus we can neither trivially check if the property is readonly, nor add the ZEND_REF_ADD_TYPE_SOURCE when the value is converted to a reference.

We could specifically fix this for ArrayIterator, which I tried here:
master...iluuu1994:php-src:gh-10844

It's not pretty but I don't know how else we could solve this without modifying both the iterator API and the internals of spl_object quite heavily.

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 17, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 17, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 17, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 19, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 24, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
@iluuu1994 @nielsdos @varrg and others