php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79447 Serializing uninitialized typed properties with __sleep should not throw
Submitted: 2020-04-03 15:06 UTC Modified: 2020-04-07 13:00 UTC
Votes:7
Avg. Score:5.0 ± 0.0
Reproduced:7 of 7 (100.0%)
Same Version:7 (100.0%)
Same OS:4 (57.1%)
From: nicolasgrekas@php.net Assigned:
Status: Closed Package: Scripting Engine problem
PHP Version: 7.4.4 OS:
Private report: No CVE-ID: None
 [2020-04-03 15:06 UTC] nicolasgrekas@php.net
Description:
------------
This is a follow up of https://bugs.php.net/bug.php?id=79002

The Symfony+Doctrine community is learning to use uninitialized properties, and we're having a bad time with __sleep().
The behavior implemented in https://github.com/php/php-src/commit/846b6479537a112d1ded725e6484e46462048b35 forbids serializing arbitrary objects (e.g.for hashing purpose). This forces us to catch and ignore "Throwable", which in turn might hide legit errors that ppl do need to see during development.

Here is an example issue https://github.com/doctrine/common/issues/886 where this is discussed, originating from https://github.com/doctrine/orm/issues/8030, which in turns generates PRs like https://github.com/symfony/symfony/pull/36336

All this activity would disappear and things would work seamlessly if the engine would just ignore uninitialized properties returned by __sleep().

On unserialize(), such properties should be unserialized back to the "uninitialized" state. This would respect the semantics of serialize/unserialize and would solve all this complexity we don't know how to deal with.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2020-04-07 13:00 UTC] nikic@php.net
I have a bit of a hard time following these threads. In the end I didn't understand a) where __sleep is actually defined/generated in the first place and b) why the __sleep returns uninitialized properties. (As a bonus question, can this usage of __sleep be migrated to __serialize?)
 [2020-04-07 13:21 UTC] ocramius@php.net
We (Doctrine) could return only the initialized properties in `__sleep`.

The only reason `__sleep` exists in proxies is to avoid serializing the whole ORM and proxy initializer closures (and transitively `PDO` too), but we could use reflection in `__sleep` to determine which properties are initialized. The performance impact is acceptable, since we're well out the 80/20 scenario.

Overall, the behavior of PHP-SRC makes sense to me, and the added strictness is welcome, so the engine throwing when `serialize($somethingWithBrokenSleep)` seems correct, although it is indeed a BC break.
 [2020-04-23 08:31 UTC] nikic@php.net
Automatic comment on behalf of nicolas.grekas@gmail.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=73d02c3b3eb8b828a1cc7ae04a4cc4f4875c3ddd
Log: Fix bug #79447
 [2020-04-23 08:31 UTC] nikic@php.net
-Status: Open +Status: Closed
 
PHP Copyright © 2001-2020 The PHP Group
All rights reserved.
Last updated: Fri Jul 10 14:01:25 2020 UTC