# Strict properties and unserialization

- Thread: https://externals.io/message/118434
- Original archive: https://news-web.php.net/php.internals/118434
- Started by Pierrick Charron on 2022-08-16T16:33:24+00:00
- Messages: 2
- RFC: Deprecate dynamic properties (Implemented (PHP 8.2)) — https://externals.io/rfc/deprecate_dynamic_properties.md
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #118434 — Pierrick Charron — 2022-08-16T16:33:24+00:00

URL: https://externals.io/message/118434#118434

Hi internals,

We (Sergey and I) would like to introduce you to a problem [1] that was
reported by Tim Düsterhus and others about the Strict Properties RFC [2]
that was implemented in PHP8.2.
The RFC missed a part about how PHP should handle unserialization of
objects with undefined properties.

Example :

class Foo {}
$foo = unserialize('O:3:"Foo":1:{s:3:"bar";i:42;}');

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).

PHP8.2beta3 will be announced this Thursday, and the next scheduled version
is supposed to be RC1. So we are pretty late in the pre-release cycle. We
have multiple options :

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.

Regards
Sergey and Pierrick

[1] https://github.com/php/php-src021-11-26./issues/9186
<https://github.com/php/php-src/issues/9186>
[2] https://wiki.php.net/rfc/deprecate_dynamic_properties

---

## #118435 — tim@bastelstu.be — 2022-08-16T17:11:27+00:00

URL: https://externals.io/message/118434#118435 · In reply to: #118434

Hi


> 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