Bug #66482
closed
Extbase persistence layer fails to create empty objects in PHP 5.6.
0%
Description
The extbase persistence layer (actually the object container) creates empty objects, by-passing the constructor, by unserializing a faked serialized empty object.
This no longer works in PHP 5.6. The unserialize() call fails fatally ("Erroneous data format").
http://php.net/manual/en/function.unserialize.php:
Changelog --------- Version Description 5.6.0 *Manipulating the serialised data by replacing C: with O: to force object instantiation without calling the constructor will now fail.*
Instead of unserialize(), public object ReflectionClass::newInstanceWithoutConstructor ( void ) must be used: http://php.net/manual/de/reflectionclass.newinstancewithoutconstructor.php (PHP >= 5.4)
TYPO3 6.2 was declared to be compatible with PHP <= 5.5, until 2 days ago, when I reported https://forge.typo3.org/issues/66468.
Now, TYPO3 6.2 is officially declared as compatible to PHP 5.3.7-5.6.x (http://typo3.org/download/). Therefore I regard this a high priority issue, as extbase is factually broken on PHP 5.6.
The error occurs in this line: https://github.com/TYPO3/TYPO3.CMS/blob/TYPO3_6-2/typo3/sysext/extbase/Classes/Object/Container/Container.php#L121
PHP 5.3 will still need the unserialize() call. PHP 5.4-5.6 can use ReflectionClass::newInstanceWithoutConstructor().
See also how Doctrine2 fixed the same issue: https://github.com/marmotz/doctrine2/commit/93c276d059b40b0783ba9a24549a8b135e257693
Updated by Klaus Bitto over 9 years ago
This is "half a duplicate" of https://forge.typo3.org/issues/66473.
Note that Bug #66473 targets TYPO3 7, but mentions "This also needs to fixed for TYPO3 6.2 though which supports PHP 5.3, in which case checking for the Serializable interface and using C instead of O could work".
Should a separate Bug targeting TYPO3 6.2 be kept?
Could "doctrine/instantiator" be introduced even in a 6.2.x release?
A Doctrine2-style fix like https://github.com/marmotz/doctrine2/commit/93c276d059b40b0783ba9a24549a8b135e257693 should certainly be possible in the next 6.2 patchlevel.
Updated by Gerrit Code Review over 9 years ago
- Status changed from New to Under Review
Patch set 1 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40522
Updated by Christian Kuhn over 9 years ago
- Description updated (diff)
- Target version changed from next-patchlevel to 6.2.15
- Sprint Focus set to Stabilization Sprint
Updated by Alexander Opitz about 9 years ago
- Target version changed from 6.2.15 to 6.2.16
Updated by Gerrit Code Review about 9 years ago
Patch set 2 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40522
Updated by Gerrit Code Review about 9 years ago
Patch set 3 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/40522
Updated by Klaus Bitto about 9 years ago
In getInstanceInternal(), first injectDependencies() is called, then initializeObject().
Doing it the other way around in getEmptyObject() would mean introducing a new bug. :) (initializeObject() might use injected objects.)
Also, there might be code depending on the fact that initializeObject is NOT called when reconstituting an object from DB. Therefore, adding initializeObject() here is a breaking change for some, while adding newInstanceWithoutConstructor() (ideally with the Doctrine workaround for final classes etc.) is only a bugfix without negative side effects.
Regarding if($classInfo->getIsInitializeable() && is_callable(array($object, 'initializeObject')): Shouldn't getIsInitializeable() have to be enough? Or in other words: If an is_callable() check is needed, isn't getIsInitializable() broken? I do see that one works on the class and the other works on the object, but the current solution still seems wrong by design.
Updated by Gerrit Code Review about 9 years ago
Patch set 4 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/40522
Updated by Gerrit Code Review about 9 years ago
Patch set 5 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/40522
Updated by Gerrit Code Review about 9 years ago
Patch set 6 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/40522
Updated by Gerrit Code Review about 9 years ago
Patch set 7 for branch TYPO3_6-2 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/40522
Updated by Helmut Hummel about 9 years ago
- Status changed from Under Review to Needs Feedback
Isn't this a duplicate of #66473 ? We should close this one or the other, I'd opt to close this one, as the other has more information gathered.
Updated by Helmut Hummel about 9 years ago
Klaus B wrote:
See also how Doctrine2 fixed the same issue: https://github.com/marmotz/doctrine2/commit/93c276d059b40b0783ba9a24549a8b135e257693
This fix isn't part of doctrine any more. Now the doctrine/instantiator code is used:
There is imho no way around that!
Updated by Klaus Bitto about 9 years ago
Helmut Hummel wrote:
Now the doctrine/instantiator code is used:
There is imho no way around that!
I guess you are right.
For me, you can close this issue in favour of #66473.
I only find it very important that this makes it into the 6.2 LTS, since you cannot keep people using PHP < 5.6 as long as they run the LTS.
Updated by Oliver Hader about 9 years ago
- Status changed from Needs Feedback to Closed