Page MenuHomePhabricator

Consider adding an unserialize_callback_func for MediaWiki to handle errors from unserialisation
Open, Needs TriagePublic

Description

This error was really confusing me, so I tried to find out what's going on.

There doesn't seem to be any documentation in the internet about what Class <class> has no unserializer really means, so I went to read the PHP source code. I learned here: https://github.com/php/php-src/blob/fb07c62f2cba5941f02a1b5b02580f3aa40c7c69/ext/standard/var_unserializer.re#L774 that this happens when the serialized class instance belongs to a class that implements Serializable (https://www.php.net/manual/en/class.serializable.php), which are serialized in a different format, but the currently existing class with that name does not implement it. __PHP_Incomplete_Class naturally doesn't implement it, so this warning happens, but this seems like a PHP bug to emit it for this case, since the real problem is that the class does not exist at all, not that it has no unserializer… And it's also worth noting that there's no warning for when the serialized class instance belongs to a class that doesn't exist; that just silently creates a __PHP_Incomplete_Class instance. So we might have those floating around our systems with no logs about it.

Here's how to reproduce the problem locally, using maintenance/shell.php:

> use MediaWiki\Extension\Notifications\Model\Event;
> class MyTempClass implements Serializable {
  public function serialize() {
    return serialize("hello");
  }
  public function unserialize($data) {
  }
}

> Event::create( [ 'type' => 'welcome', 'agent' => User::newFromName('Matma Rex'), 'extra'=>['x'=>new MyTempClass] ] )
= MediaWiki\Extension\Notifications\Model\Event {#6275}

You will now see these warnings whenever the notification is shown.

if you do the same thing with a class that is not Serializable:

> class MyTempClass2 {}
> Event::create( [ 'type' => 'welcome', 'agent' => User::newFromName('Matma Rex'), 'extra'=>['x'=>new MyTempClass2] ] )
= MediaWiki\Extension\Notifications\Model\Event {#4945}

… you will not see any warnings, but you probably should.

Reading the docs, I found that we can set a callback that is called when a serialized class instance belongs to a class that doesn't exist: https://www.php.net/manual/en/var.configuration.php#ini.unserialize-callback-func. We could probably use this to log a warning of our own, including for the case where the missing class doesn't implement Serializable (so it doesn't cause *this* warning).

I tried adding this in my LocalSettings.php:

ini_set('unserialize_callback_func', 'wfUnserializeMissingClass');
function wfUnserializeMissingClass($className) {
  MediaWiki\Logger\LoggerFactory::getInstance('unserialize')->error( 'Missing class {class}', [ 'class' => $className ] );
}

…and it works, I now get errors logged with the class name for both cases (as well as an extra PHP Warning: unserialize(): Function wfUnserializeMissingClass() hasn't defined the class it was called for from PHP, but I think that's fine). We might want to do something like this in MediaWiki.

(Final emphasis mine.)

Event Timeline

Second similar incident good comment to also capture:

Adding some biz logic in WANObjectCache to check for __PHP_Incomplete_Class returns from unserialize and log what the __PHP_Incomplete_Class_Name and key are would maybe be nice.


Treating an __PHP_Incomplete_Class result as a cache miss would likely work too.