Uncaught Exception: Serialization of 'Closure' is not allowed in /wordpress/drop-ins/object-cache.php

[sisaacrussell]

Bug Report

• [X] Yes, I reviewed the contribution guidelines.
• [X] Yes, more specifically, I reviewed the guidelines on how to write clear bug reports.

When hosting on Pressable using their memcache object caching (presumably the same issue exists on other hosts with object caching but I cannot test that), and running PHP 8.1 or above, the following error is thrown when accessing wp-admin dashboard:

PHP Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in /wordpress/drop-ins/object-cache.php:974 Stack trace: #0 /wordpress/drop-ins/object-cache.php(974): serialize(Array) 
#1 /wordpress/drop-ins/object-cache.php(617): WP_Object_Cache->get_data_size(Array) 
#2 /wordpress/drop-ins/object-cache.php(95): WP_Object_Cache->set('backtrace_8192l...', Array, 'bh-wp-logger', 86400) 
#3 /srv/htdocs/wp-content/plugins/bh-wp-autologin-urls/vendor-prefixed/brianhenryie/bh-wp-logger/src/api/class-api.php(305): wp_cache_set('backtrace_8192l...', Array, 'bh-wp-logger', 86400) 
#4 /srv/htdocs/wp-content/plugins/bh-wp-autologin-urls/vendor-prefixed/brianhenryie/bh-wp-logger/src/api/class-api.php(329): BrianHenryIE\WP_Autologin_URLs\WP_Logger\API\API->get_backtrace('8192ltrimpassin...', NULL) 
#5 /srv/htdocs/wp-content/plugins/bh-wp-autologin-urls/vendor-prefixed/brianhenryie/bh-wp-logger/src/php/class-php-error-handler.php(187): BrianHenryIE\WP_Autologin_URLs\WP_Logger\API\API->is_backtrace_contains_plugin('8192ltrimpassin...') 
#6 /srv/htdocs/wp-content/plugins/bh-wp-autologin-urls/vendor-prefixed/brianhenryie/bh-wp-logger/src/php/class-php-error-handler.php(99): BrianHenryIE\WP_Autologin_URLs\WP_Logger\PHP\PHP_Error_Handler->is_related_error(8192, 'ltrim(): Passin...', '/wordpress/core...', 4494) 
#7 [internal function]: BrianHenryIE\WP_Autologin_URLs\WP_Logger\PHP\PHP_Error_Handler->plugin_error_handler(8192, 'ltrim(): Passin...', '/wordpress/core...', 4494) 
#8 /wordpress/core/6.4.3/wp-includes/formatting.php(4494): ltrim(NULL) 
#9 /wordpress/core/6.4.3/wp-includes/formatting.php(4620): esc_url(NULL, NULL, 'db') 
#10 /wordpress/core/6.4.3/wp-includes/formatting.php(4602): sanitize_url(NULL, NULL) #11 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Authentication/Authentication.php(852): esc_url_raw(NULL) 
#12 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Util/Method_Proxy_Trait.php(25): Google\Site_Kit\Core\Authentication\Authentication->inline_js_base_data(Array) 
#13 /wordpress/core/6.4.3/wp-includes/class-wp-hook.php(324): Google\Site_Kit\Core\Authentication\Authentication->Google\Site_Kit\Core\Util\{closure}(Array) #14 /wordpress/core/6.4.3/wp-includes/plugin.php(205): WP_Hook->apply_filters(Array, Array) 
#15 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(739): apply_filters('googlesitekit_i...', Array) #16 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(356): Google\Site_Kit\Core\Assets\Assets->get_inline_base_data() 
#17 [internal function]: Google\Site_Kit\Core\Assets\Assets->Google\Site_Kit\Core\Assets\{closure}('googlesitekit-b...') 
#18 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Script_Data.php(51): call_user_func(Object(Closure), 'googlesitekit-b...') 
#19 [internal function]: Google\Site_Kit\Core\Assets\Script_Data->Google\Site_Kit\Core\Assets\{closure}('googlesitekit-b...') 
#20 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Asset.php(129): call_user_func(Object(Closure), 'googlesitekit-b...') 
#21 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1016): Google\Site_Kit\Core\Assets\Asset->before_print() 
#22 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1025): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array) 
#23 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1025): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array) 
#24 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(1025): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array) 
#25 /srv/htdocs/wp-content/plugins/google-site-kit/includes/Core/Assets/Assets.php(155): Google\Site_Kit\Core\Assets\Assets->run_before_print_callbacks(Object(WP_Scripts), Array) 
#26 /wordpress/core/6.4.3/wp-includes/class-wp-hook.php(324): Google\Site_Kit\Core\Assets\Assets->Google\Site_Kit\Core\Assets\{closure}('') 
#27 /wordpress/core/6.4.3/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array) 
#28 /wordpress/core/6.4.3/wp-includes/plugin.php(517): WP_Hook->do_action(Array) 
#29 /wordpress/core/6.4.3/wp-admin/admin-header.php(146): do_action('admin_print_scr...') 
#30 /wordpress/core/6.4.3/wp-admin/admin.php(239): require_once('/wordpress/core...') 
#31 {main} thrown in /wordpress/drop-ins/object-cache.php on line 974

Switching to PHP 7.4 or deactivating the Magic Emails & Autologin URLs plugins resolves the issue.

[BrianHenryIE]

Interesting.

There’s a first bug in Google Site Kit.

This plugin is using a logger library, brianhenryie/bh-wp-logger, which checks every error to see is the error related to this plugin.

As part of that, it’s trying to cache the backtrace so it’s not repeatedly run unnecessarily. It’s probably crashing on the first run.

So 1: fix the caching issue 2: try add some error handling inside the error handler so my code doesn’t obfuscate other plugins errors 3: there’ll still be the root error

I’ll try get this fixed this week. I just started vacation yesterday so will enjoy the first few days and then should have the time to look.

Thanks for the report

[sisaacrussell]

Hope you had a great vacay! Any update on this issue? I'd really like to get this site off 7.4 and still continue using this plugin.

[BrianHenryIE]

I have just managed to merge all my outstanding changes and I'm working towards your issues now.

Trying to reproduce:

wp core update --version=6.4.3
wp plugin install google-site-kit --version=1.122.0 # The version at March 23
wp plugin activate google-site-kit
sphp 8.1

I did not manage to immediately reproduce the error.

I think I just need to loop over the array before this line and remove anything that is a closure:

https://github.com/BrianHenryIE/bh-wp-logger/blob/ba309cc8d1e167354c3835e53471201b7cc7cf38/src/api/class-api.php#L301

Obviously, it would be nicer if I could Xdebug my way through, write a test and confirm it's fixed.

I've spent 3+ hours on this project three evenings this week. More to come!

[BrianHenryIE]

I've written the test and code to recursively check for closures and remove them before serializing.

I still have some code changes in another library, bh-wp-private-uploads, to review and upload before I can make a new build here.

To say: I'm still working on it.

[sisaacrussell]

I'm curious if that error is only thrown if the Google Site Kit plugin is connected (or perhaps only if connected in a specific configuration) and that's why it doesn't show in your tests.

Thank you for your time and work on this project.

[BrianHenryIE]

What's happening is esc_url_raw( null ) is being called here: site-kit-wp Authentication.php#L852, which hits wordpress formatting.php#L4494, where it calls ltrim( null ).

On PHP 8, that does not output any error:

$ /opt/homebrew/Cellar/php@8.0/8.0.30_1/bin/php -r "ltrim(null);"   

But 8.1 outputs a deprecation notice:

$ /opt/homebrew/Cellar/php@8.1/8.1.26/bin/php -r "ltrim(null);"
PHP Deprecated:  ltrim(): Passing null to parameter #1 ($string) of type string is deprecated in Command line code on line 1

My logging library listens for all errors on the site to identify which are caused by its own plugin so it can appropriately log them for the user, see bh-wp-logger class-php-error-handler.php#L183. Part of this involves running a backtrace, which AIUI (I have not profiled, TBH) is a bit expensive, and I would have many plugins on one site running with this logger, so to avoid processing the same backtrace ten times, I'm caching it.

As you see, in the implementation of wp_cache_set() it's using serialize(). But if we try to serialize a closure we get an exception:

$closure_function = function () {
    throw new \Exception( 'Thrown inside closure' );
};

try {
    $closure_function();
} catch ( \Exception $exception ) {
    $backtrace_containing_closure = $exception->getTrace();
    serialize( $backtrace_containing_closure );
    // Serialization of 'Closure' is not allowed
}

I think inside Google Site Kit, it's applying a filter: Assets.php#L739C26-L739C56 which was added at Authentication.php#L280 and Site Kit seems to be using some funky closures pattern for all its actions/filters: MethodProxyTrait::get_method_proxy().

So there is a bug to fix in my code, for sure, thank you. Maybe Site Kit should check it's dealing with a URL before doing any processing on it. But WordPress's esc_url() is already doing a check for a zero length string, '' == $url, which if it were changed to empty( $url ) would also work to fix this. You should consider opening a trac ticket and sending a patch – it would be a nice bite-size fix to get yourself the Core Contributor badge if you don't already have it.

I can tell that you're running your site with WP_DEBUG true, which turns on reporting for all errors, including deprecation warnings, see load.php#L577. If you were to disable that, PHP will never fire the error and things should work. Personally, I like to run sites with WP_DEBUG enabled and I like to fix all the errors that I see.

Thanks for the kind words. This is a plugin I wrote for the job I had a few years ago. Now I just maintain it as a hobby, because I think it should be on every site!

Edit: as I think about it more, wp_cache_set() isn't the appropriate function to use. I want to cache the result only for the current request. There's no need for it to use memcache.

[BrianHenryIE]

Should be fixed in the latest release: 2.4.0, also on WordPress.org.

Sorry for the delay, I was trying to get more thorough E2E testing running but didn't quite manage to.