Opened 3 weeks ago
#63594 new defect (bug)
Bug/Broken State possible in Multisite Caching Code: update_network_option
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | 6.8 |
Component: | Options, Meta APIs | Keywords: | |
Focuses: | Cc: |
Description
I ran into an issue at one point with the caching code and how the "when to refresh from database" logic is handled.
Here is how to reproduce:
- Have WordPress running with a Cache backend of your choice
- Create/Update a network option via update_network_option() or add_network_option()
- Now, delete the network option directly from the database
It is now impossible to update the network option, via update_network_option() ... until the cache is also cleared because of how the login runs in the method. Basically the following happens:
First, it attempts to get the option:
$old_value = get_network_option( $network_id, $option );
And it will only 'add' the option, if it didn't exist already:
if ( $value === $old_value || maybe_serialize( $value ) === maybe_serialize( $old_value ) ) {
return false;
}
if ( false === $old_value ) {
return add_network_option( $network_id, $option, $value );
}
So far so good (in a way) ... but then the next section of the code is where a problem happens ... here's that section as a whole:
$notoptions_key = "$network_id:notoptions";
$notoptions = wp_cache_get( $notoptions_key, 'site-options' );
if ( is_array( $notoptions ) && isset( $notoptions[ $option ] ) ) {
unset( $notoptions[ $option ] );
wp_cache_set( $notoptions_key, $notoptions, 'site-options' );
}
if ( ! is_multisite() ) {
$result = update_option( $option, $value, false );
} else {
$value = sanitize_option( $option, $value );
$serialized_value = maybe_serialize( $value );
$result = $wpdb->update(
$wpdb->sitemeta,
array( 'meta_value' => $serialized_value ),
array(
'site_id' => $network_id,
'meta_key' => $option,
)
);
if ( $result ) {
$cache_key = "$network_id:$option";
wp_cache_set( $cache_key, $value, 'site-options' );
}
}
So the problem in above lies in this underlying condition ... following the logic for at this point (for multisite specifically). If the cache entry was found (which it was) ... it's going to immediately drop into section that will be doing the $wpdb->update()
... which is going to fail because there is no option/row to update.
Since it failed, the rest of the function just ends up being skipped, and it returns false.
So basically:
- update_network_option ... will in fact create the option if it didn't exist already
- BUT: if an old/stale cached version of the data exists, it will still try to 'update' the option instead ... which will fail since it doesn't really exist in the database.
In my case this happened (and has happened) multiple times due to restoring/automating some database updates, that didn't also flush the cache.
There are a couple solutions to this. One would be to update this code to use wpdb->replace
instead of update ... but that's potentially a bigger change.
A very simple solution to this would simply be to change this code:
if ( $result ) {
$cache_key = "$network_id:$option";
wp_cache_set( $cache_key, $value, 'site-options' );
}
To this:
if ( $result ) {
$cache_key = "$network_id:$option";
wp_cache_set( $cache_key, $value, 'site-options' );
} else {
return add_network_option( $network_id, $option, $value );
}
Which would basically be one additional 'attempt'. That if the code gets here, tries to 'update' the option and can't for some reason, it goes "Huh, well, let's assume that failed because it didn't exist. And so lets give a one-time chance at just adding this instead