Performance Lab Led to a Critical Error with Site Health

[bobbyferran]

Bug Description

Steps to reproduce

1. Go to 'tools'
2. Click on 'site health'
3. Scroll down to '....' (n/a)
4. See error: There has been a critical error on this website.

Screenshots

Additional Context

• PHP Version: 8.2.18
• OS: [iOS]
• Browser: [Firefox]
• Plugin Version: [e.g. 22]
• Device: [Macbook]

<This only occurs with site health. Otherwise, the site works and site health became available again when I disabled performance lab.>

[bobbyferran]

Plugin Version: Performance Lab (version 3.1.0)

[bobbyferran]

[westonruter]

If you deactivate Performance Lab then the error goes away?

We'll need to get more information from you about what is going on. Specifically we'll need the specific PHP error that is occuring. Can you get that from the error log?

[bobbyferran]

Yes, and then site health works again.
This is the error log:

Error Details
=============
An error of type E_ERROR was caused in line 106 of the file /home3/flowergu/public_html/wp-content/plugins/performance-lab/includes/site-health/audit-autoloaded-options/helper.php. Error message: Uncaught TypeError: strlen(): Argument #1 ($string) must be of type string, array given in /home3/flowergu/public_html/wp-content/plugins/performance-lab/includes/site-health/audit-autoloaded-options/helper.php:106
Stack trace:
#0 /home3/flowergu/public_html/wp-content/plugins/performance-lab/includes/site-health/audit-autoloaded-options/helper.php(22): perflab_aao_autoloaded_options_size()
#1 /home3/flowergu/public_html/wp-admin/includes/class-wp-site-health.php(194): perflab_aao_autoloaded_options_test()
#2 /home3/flowergu/public_html/wp-admin/includes/class-wp-site-health.php(145): WP_Site_Health->perform_test('perflab_aao_aut...')
#3 /home3/flowergu/public_html/wp-includes/class-wp-hook.php(324): WP_Site_Health->enqueue_scripts('site-health.php')
#4 /home3/flowergu/public_html/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
#5 /home3/flowergu/public_html/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#6 /home3/flowergu/public_html/wp-admin/admin-header.php(118): do_action('admin_enqueue_s...', 'site-health.php')
#7 /home3/flowergu/public_html/wp-admin/site-health.php(96): require_once('/home3/flowergu...')
#8 {main}
thrown

[westonruter]

Thank you. I can see this is a bug introduced in #1179. The code in question:

performance/includes/site-health/audit-autoloaded-options/helper.php

Lines 105 to 107 in be4c00a

	foreach ( $all_options as $option_name => $option_value ) {
	$total_length += strlen( $option_value );
	}

The issue is that you have an autoloaded option which is an array (which is perfectly fine) but the code is assuming all of the options are strings (I guess since previously it was only working with serialized data).

Working on a fix...

[westonruter]

Interesting. In testing locally, I'm seeing that wp_load_alloptions() only returns an array of strings, even when there are autoloaded arrays/objects they are returned as serialized data.

@bobbyferran Do you have an external object cache perhaps?

[bobbyferran]

I have Docket Cache but this was previously installed with performance lab and there was not this issue. This issue just started today without adding any new plugins. I also have WP Fastest Cache and the Bluehost assets only cache.

[westonruter]

OK, thanks. I'm able to reproduce the issue with Docket Cache. It seems when Docket Cache is active it results in wp_load_alloptions() returning an array of mixed values, including arrays. I do not see this issue when I try another object cache plugin, namely SQLite Object Cache. When I check WordPress core, I see that it is intending to store an array of serialized (string) values in the alloptions cache:

if ( 'yes' === $autoload ) {
	$alloptions            = wp_load_alloptions( true );
	$alloptions[ $option ] = $serialized_value;
	wp_cache_set( 'alloptions', $alloptions, 'options' );
}

So this makes me think that indeed Docket Cache's object cache implementation is incorrect, as it seems to be storing unserialized values instead of serialized ones. I think this should be reported as a bug to Docket Cache, although we should also harden Performance Lab against such object cache implementation bugs.

In the mean time, here is some workaround PHP plugin code that "fixes" the Docket Cache compatibility problem:

add_filter(
	'alloptions',
	static function ( array $alloptions ): array {
		return array_map(
			static function ( $value ): string {
				if ( ! is_string( $value ) ) {
					return serialize( $value );
				} else {
					return $value;
				}
			},
			$alloptions
		);
	}
);

[bobbyferran]

Ok, thank you. I will also report this issue to Docket Cache. Thank you again.

[westonruter]

@bobbyferran Great. Could you share the link to your Docket Cache issue when you've reported?

Also, I've opened #1238 to harden Performance Lab against this issue.

[mukeshpanchal27]

@westonruter, does this plugin issue alter the behaviour of WP functions in a way that might break the default WP functionality?

[westonruter]

@mukeshpanchal27 Yes, that seems likely.

[nawawi]

Hi there,

does this plugin issue alter the behaviour of WP functions in a way that might break the default WP functionality?

Nope. It store cache as plain PHP to get performance benefit from OPcache.

You may do a quick fix by adding maybe_serialize:

File: includes/site-health/audit-autoloaded-options/helper.php
Line: 106

Before:

foreach ( $all_options as $option_name => $option_value ) {
    $total_length += strlen( $option_value );
}

Fix:

foreach ( $all_options as $option_name => $option_value ) {
    $total_length += strlen( maybe_serialize($option_value));
}

Thanks.

[westonruter]

@nawawi Right, but WordPress core is returning an array of serialized options when calling wp_load_alloptions(). Therefore, to ensure there are no compatibility problems, it seems Docket Cache should also serialize the option values for consistency.

We implemented a similar workaround in #1238.

[nawawi]

Alright then, thanks.