Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118435
Message-ID: <33d2dab3-baf7-c275-69af-8a3eb4581f64@bastelstu.be>
Date: Tue, 16 Aug 2022 19:11:27 +0200
MIME-Version: 1.0
Content-Language: en-US
To: Pierrick Charron <pierrick@php.net>, Sergey Panteleev <sergey@php.net>,
 PHP internals <internals@lists.php.net>
References: <CAOfKkdSbByKtrP=hnjmhtmLHiU+V-46wx2ojrQEt8EVtOP8ZtQ@mail.gmail.com>
In-Reply-To: <CAOfKkdSbByKtrP=hnjmhtmLHiU+V-46wx2ojrQEt8EVtOP8ZtQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Strict properties and unserialization
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=c3=bcsterhus?=)

Hi

On 8/16/22 18:33, Pierrick Charron wrote:
> In this case PHP will not trigger any deprecation warnings but it probably
> should. It was something that was not thought about while writing the RFC
> (at least the RFC doesn't mention anything about this).

Note that for native classes (e.g. the Randomizer from my issue), 
creating an unknown property is not just a deprecation, but an Error if 
'@strict-properties' is set:

> Fatal error: Uncaught Error: Cannot create dynamic property Random\Randomizer::$foo in php-src/test.php:4
> Stack trace:
> #0 {main}
>   thrown in php-src/test.php on line 4

---

> First we could just do nothing in PHP8.2 about this and leave it like that
> and only fix it in PHP8.3. This is probably the way we should do it if we
> want to stick to the initial PHP8.2 release cycle calendar.
> The second option would be to fix this in PHP8.2 before RC1. We don't have
> any idea on how big the patch is and it may influence the final decision.
> If patching it for 8.2 is the best way to do, we may want to add another
> Beta (Beta4) and delay the RC1 release by 2 weeks.
>
> We would like to hear your thoughts about this issue.

The above example (ZEND_ACC_NO_DYNAMIC_PROPERTIES) technically even 
affects PHP 8.0 [1]. As such this is not a new issue, but I would 
recommend treating this as an actual bug and fixing this in PHP 8.2, 
because my understanding is that this is intended to protect internal 
classes:

https://github.com/php/php-src/commit/653e4ea1c57def2d5cd75b7da9e3943a841b7d6c

and

https://github.com/php/php-src/pull/5533#discussion_r422474377

It might be useful to include nikic in the discussion here?

[1] In 8.0 none of the native classes having that flag are 
unserializable, though.

Best regards
Tim Düsterhus