php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #79002 Serializing uninitialized typed properties with __sleep makes unserialize throw
Submitted: 2019-12-19 16:10 UTC Modified: 2020-01-09 11:38 UTC
From: tandre@php.net Assigned: nikic (profile)
Status: Closed Package: Scripting Engine problem
PHP Version: 7.4.1 OS: Any
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: tandre@php.net
New email:
PHP Version: OS:

 

 [2019-12-19 16:10 UTC] tandre@php.net
Description:
------------
When __sleep includes uninitialized typed properties, serialize() will succeed (behaves as if the value was null), but unserialize() will cause a TypeError to be thrown if the value wasn't nullable.

I didn't see any mention of how uninitialized typed properties should behave for __sleep in the RFC or RFC/PR discussion.

Is the current behavior in 7.4 and 8.0-dev for __sleep on uninitialized properties by design, or is it an edge case that was overlooked? It seems surprising that data serializes successfully, but throws when it gets unserialized for the same class declaration.

I checked https://wiki.php.net/rfc/typed_properties_v2 , https://externals.io/message/102333 , and https://externals.io/message/103148 , and didn't see any references to __sleep (just "I wouldn't lose sleep
if we wanted to keep the GA date where it is.")
I also didn't see any mention of __sleep in php/php-src#3734 , or in the changes of the PR. (or in bugs.php.net)

Noticed in https://github.com/igbinary/igbinary/pull/250#discussion_r355812953

Test script:
---------------
<?php
class HasProp {
    public int $x;
    public function __sleep() { return ['x']; }
}
$x = new HasProp();
// string(28) "O:7:"HasProp":1:{s:1:"x";N;}"
var_dump($s = serialize($x));
// Fatal error: Uncaught TypeError: Typed property HasProp::$x must be int, null used in ...
unserialize($s);


Expected result:
----------------
Unserialize() should not throw, and should generate an instance of HasProp, with an uninitialized HasProp.

Ideally, serialize() should skip over the property $x. This would make it match the original data's representation for both typed nullable and non-nullable properties.



Actual result:
--------------
This throws a TypeError when unserializing the data it just serialized

Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2019-12-19 22:53 UTC] requinix@php.net
Note that serialize() is already notice-ing about this. https://3v4l.org/PCLm3

On the one hand, the current behavior should be fine. (Or maybe escalate it to a warning instead of just a notice.) The property is not set, should work the same way whether it's typed or not.

On the other hand, serialize() could check that the property is defined but typed and uninitialized, then warn and *not* include it in the serialized data. There is precedence for special cases like this.
 [2019-12-20 00:26 UTC] tandre@php.net
The following pull request has been associated:

Patch Name: Change __sleep serialization for uninitialized typed properties
On GitHub:  https://github.com/php/php-src/pull/5027
Patch:      https://github.com/php/php-src/pull/5027.patch
 [2020-01-06 17:49 UTC] nikic@php.net
Fixed with https://github.com/php/php-src/commit/846b6479537a112d1ded725e6484e46462048b35 by moving the exception already to the serialization.

If it turns out there are compelling reasons to allow uninit props in __sleep() we can easily relax this in future, but for now this is the conservative choice.
 [2020-01-09 11:38 UTC] nikic@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: nikic
 [2020-01-09 11:38 UTC] nikic@php.net
Forgot to close.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Nov 02 09:01:28 2024 UTC