| 2022-12-17 01:47:40 |
Bryce Harrington |
description |
[Impact]
When you serialize an object in php7.4 on Focal, it checks to see if a magic __sleep() function is present, and if so, calls it before the object is serialized. Its intention is to give you an opportunity to close database connections or drop unnecessary data before you serialize it to disk.
Now, in php 7.4.2 a commit was introduced this implemented this behaviour, and they had implemented throwing an error when you try serialize uninitialised typed data to match the semantics of what happens if you directly access an uninitialized typed attribute of an object.
This caused some problems, namely, you had to define a exception handler when you attempted to serialize, and you could end up catching actually important errors too early instead of the intended uninitialized data errors.
The community opened some bugs about this upstream, and it was decided to change the behaviour such that serialize with __sleep() has the same semantics as serialize without __sleep(), namely, removing the error that can be thrown.
[Testcase]
Start a fresh Focal VM, and follow the instructions below.
1) sudo apt install docker.io php7.4 php7.4-xml php7.4-pgsql make
2) curl -1sLf 'https://dl.cloudsmith.io/public/symfony/stable/setup.deb.sh' | sudo -E bash
3) sudo apt install symfony-cli composer
4) git clone https://github.com/2ec0b4/symfony-proxy-issue.git
5) cd symfony-proxy-issue
6) composer install --optimize-autoloader
6) make start
7) curl http://127.0.0.1:8000
...
Error:
Typed property Proxies\__CG__\App\Entity\Store::$ must not be accessed before initialization (in __sleep)
at vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:626
at serialize()
(vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:626)
at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->getCacheKey()
(vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:327)
at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize()
(vendor/symfony/serializer/Serializer.php:191)
at Symfony\Component\Serializer\Serializer->denormalize()
(src/Controller/DefaultController.php:47)
at App\Controller\DefaultController->index()
(vendor/symfony/http-kernel/HttpKernel.php:146)
at Symfony\Component\HttpKernel\HttpKernel->handleRaw()
(vendor/symfony/http-kernel/HttpKernel.php:68)
at Symfony\Component\HttpKernel\HttpKernel->handle()
(vendor/symfony/http-kernel/Kernel.php:201)
at Symfony\Component\HttpKernel\Kernel->handle()
(public/index.php:25)
...
There is a test package available in the following ppa:
https://launchpad.net/~mruffell/+archive/ubuntu/sf350406-test
If you install the test package, the issue no longer occurs and the page renders
successfully.
$ curl -I http://127.0.0.1:8000
HTTP/1.1 200 OK
[Where problems could occur]
We are changing how object serialization occurs within __sleep(), such that it no longer throws an error when we try and serialize uninitialized types.
The impact should be minimal, since it brings the exact same semantics developers expect when they serialize objects without __sleep() to objects that use __sleep().
In the forward case of object serialization, it means we will no longer throw an error, so any exception handlers that have been implemented in users projects for this purpose won't be called anymore, due to serialization always returning success. Apart from some unreachable code, there is no detrimental effect.
In the backward case of creating an object from serialized data, there is a risk of a small regression or more so a change in behaviour. When the object is unserialized, uninitialized attributes will be replaced with default values as mentioned in the commit text:
> As in the non-__sleep() case, unserialize(serialize($x)) identity
> may not be preserved due to replacement of uninitialized/unset
> properties with default values. Fixing this will require changes to
> the serialization format.
If your code makes assumptions on uninitialized attributes vs attributes with default values, you are likely in trouble anyway, but this could cause a regression for some users.
The changes only affect when __sleep() is present, and no changes are made to any code paths where __sleep() is not defined, so the most executed path remains unchanged, and limits regressions to users that use __sleep() only.
[Other info]
Information about the __sleep() method can be found in the upstream documentation:
https://www.php.net/manual/en/language.oop5.magic.php#object.sleep
This issue was introduced upstream in php 7.4.2 in commit:
commit 846b6479537a112d1ded725e6484e46462048b35
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Fri, 3 Jan 2020 15:35:10 +0100
Subject: Throw Error when referencing uninit typed prop in __sleep
Link: https://github.com/php/php-src/commit/846b6479537a112d1ded725e6484e46462048b35
The issue was then fixed upstream in php 7.4.6 in commit:
commit 73d02c3b3eb8b828a1cc7ae04a4cc4f4875c3ddd
From: Nicolas Grekas <nicolas.grekas@gmail.com>
Date: Thu, 16 Apr 2020 00:11:38 +0200
Subject: Fix bug #79447
Link: https://github.com/php/php-src/commit/73d02c3b3eb8b828a1cc7ae04a4cc4f4875c3ddd
The fix cherry picks cleanly with no modifications necessary, apart from dropping the NEWS hunk. |
[Impact]
When you serialize an object in php7.4 on Focal, it checks to see if a magic __sleep() function is present, and if so, calls it before the object is serialized. Its intention is to give you an opportunity to close database connections or drop unnecessary data before you serialize it to disk.
Now, in php 7.4.2 a commit was introduced that implemented this behaviour, and they had implemented throwing an error when you try to serialize uninitialised typed data to match the semantics of what happens if you directly access an uninitialized typed attribute of an object.
This caused some problems, namely, you had to define an exception handler when you attempted to serialize, and you could end up catching actually important errors too early instead of the intended uninitialized data errors.
The community opened some bugs about this upstream, and it was decided to change the behaviour such that serialize with __sleep() has the same semantics as serialize without __sleep(), namely, removing the error that can be thrown.
[Testcase]
Start a fresh Focal VM, and follow the instructions below.
1) sudo apt install docker.io php7.4 php7.4-xml php7.4-pgsql make
2) curl -1sLf 'https://dl.cloudsmith.io/public/symfony/stable/setup.deb.sh' | sudo -E bash
3) sudo apt install symfony-cli composer
4) git clone https://github.com/2ec0b4/symfony-proxy-issue.git
5) cd symfony-proxy-issue
6) composer install --optimize-autoloader
6) make start
7) curl http://127.0.0.1:8000
...
Error:
Typed property Proxies\__CG__\App\Entity\Store::$ must not be accessed before initialization (in __sleep)
at vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:626
at serialize()
(vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:626)
at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->getCacheKey()
(vendor/symfony/serializer/Normalizer/AbstractObjectNormalizer.php:327)
at Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize()
(vendor/symfony/serializer/Serializer.php:191)
at Symfony\Component\Serializer\Serializer->denormalize()
(src/Controller/DefaultController.php:47)
at App\Controller\DefaultController->index()
(vendor/symfony/http-kernel/HttpKernel.php:146)
at Symfony\Component\HttpKernel\HttpKernel->handleRaw()
(vendor/symfony/http-kernel/HttpKernel.php:68)
at Symfony\Component\HttpKernel\HttpKernel->handle()
(vendor/symfony/http-kernel/Kernel.php:201)
at Symfony\Component\HttpKernel\Kernel->handle()
(public/index.php:25)
...
There is a test package available in the following ppa:
https://launchpad.net/~mruffell/+archive/ubuntu/sf350406-test
If you install the test package, the issue no longer occurs and the page renders
successfully.
$ curl -I http://127.0.0.1:8000
HTTP/1.1 200 OK
[Where problems could occur]
We are changing how object serialization occurs within __sleep(), such that it no longer throws an error when we try and serialize uninitialized types.
The impact should be minimal, since it brings the exact same semantics developers expect when they serialize objects without __sleep() to objects that use __sleep().
In the forward case of object serialization, it means we will no longer throw an error, so any exception handlers that have been implemented in users projects for this purpose won't be called anymore, due to serialization always returning success. Apart from some unreachable code, there is no detrimental effect.
In the backward case of creating an object from serialized data, there is a risk of a small regression or more so a change in behaviour. When the object is unserialized, uninitialized attributes will be replaced with default values as mentioned in the commit text:
> As in the non-__sleep() case, unserialize(serialize($x)) identity
> may not be preserved due to replacement of uninitialized/unset
> properties with default values. Fixing this will require changes to
> the serialization format.
If your code makes assumptions on uninitialized attributes vs attributes with default values, you are likely in trouble anyway, but this could cause a regression for some users.
The changes only affect when __sleep() is present, and no changes are made to any code paths where __sleep() is not defined, so the most executed path remains unchanged, and limits regressions to users that use __sleep() only.
[Other info]
Information about the __sleep() method can be found in the upstream documentation:
https://www.php.net/manual/en/language.oop5.magic.php#object.sleep
This issue was introduced upstream in php 7.4.2 in commit:
commit 846b6479537a112d1ded725e6484e46462048b35
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Fri, 3 Jan 2020 15:35:10 +0100
Subject: Throw Error when referencing uninit typed prop in __sleep
Link: https://github.com/php/php-src/commit/846b6479537a112d1ded725e6484e46462048b35
The issue was then fixed upstream in php 7.4.6 in commit:
commit 73d02c3b3eb8b828a1cc7ae04a4cc4f4875c3ddd
From: Nicolas Grekas <nicolas.grekas@gmail.com>
Date: Thu, 16 Apr 2020 00:11:38 +0200
Subject: Fix bug #79447
Link: https://github.com/php/php-src/commit/73d02c3b3eb8b828a1cc7ae04a4cc4f4875c3ddd
The fix cherry picks cleanly with no modifications necessary, apart from dropping the NEWS hunk. |
|
