Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fatal Error occurs when calling msgpack_unpack() function in PHP 7.4 or higher environment. #2299

Closed
suzunone opened this issue Mar 10, 2021 · 2 comments · Fixed by #2384
Closed

Comments

@suzunone
Copy link

Hello,
Fatal Error occurs when calling msgpack_unpack() function in PHP 7.4 or higher environment.

The msgpack_unpack() function can be used if pecl msgpack is enabled.
https://pecl.php.net/package/msgpack

I encountered an issue with the following code:

$now = \Carbon\Carbon::parse('2018-06-01');
$message = @msgpack_pack($now);
$copy = msgpack_unpack($message);

Carbon version: 2.46.0

PHP version: 7.4.14

pecl msgpack version:msgpack-2.1.2
https://pecl.php.net/package/msgpack

I expected to get:

PHP Fatal error:  Uncaught Error: Invalid serialization data for DateTime object in ./vendor/nesbot/carbon/src/Carbon/Traits/Serialization.php:131
Stack trace:
#0 ./vendor/nesbot/carbon/src/Carbon/Traits/Serialization.php(131): DateTime->__wakeup()
#1 [internal function]: Carbon\Carbon->__wakeup()
#2 ./test.php(6): msgpack_unpack('\x84\xC0\xADCarbon\\Carbo...')
#3 {main}
  thrown in ./vendor/nesbot/carbon/src/Carbon/Traits/Serialization.php on line 131

But I actually get:

$now  = $copy;

Thanks!

@kylekatarnls
Copy link
Collaborator

Hello, 👋

Does the following work:

$now = new DateTime('2018-06-01');
$message = @msgpack_pack($now);
$copy = msgpack_unpack($message);

If it does not, then it would be way more relevant to fix the error in msgpack as it impact any DateTime sub-class.

Else it means it impact only classes that extends DateTime and override parent::__wakeup() which is still a normal PHP case that should work in PHP >= 7.4 as per the unserialize() test:

$this->assertEquals(Carbon::now(), unserialize(serialize(Carbon::now())));

Replacing __wakeup with __construct as proposed #2300 would be a work-around that may cause other issues. DateTime::__wakeup is supposed to be called on unserialization and I'm a bit reluctant to short-circuit it just to please a minor package.

Finding what exactly msgpack is failing on here may help to get a better long-term fix.

@suzunone
Copy link
Author

Hello,
Thanks for your reply.

If it use the vanilla DateTime class instead of Carbon as suggested, it works fine.

<?php

$now = new DateTime('2018-06-01');
$message = @msgpack_pack($now);
$copy = msgpack_unpack($message);

var_dump($copy);
test.php:7
class DateTime#2 (3) {
  public $date =>
  string(26) "2018-06-01 00:00:00.000000"
  public $timezone_type =>
  int(3)
  public $timezone =>
  string(10) "America/Toronto"
}

If it call __wakeup() method directly, as in the following example, will get a Fatal error.

<?php

$now = new DateTime('2018-06-01');
$now->__wakeup();
PHP Fatal error:  Uncaught Error: Invalid serialization data for DateTime object in ./test.php:4
Stack trace:
#0 /data/webapp/test.php(4): DateTime->__wakeup()
#1 {main}
  thrown in /data/webapp/test.php on line 4

This error did not occur in 7.3 or earlier.

Pecl msgpack calls __wakeup() method when msgpack_unpack() is called, as expected.
I haven't tried other libraries than msgpack, but I think the same problem may occur with similar serialize libraries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants