[wp-trac] [WordPress Trac] #63594: Bug/Broken State possible in Multisite Caching Code: update_network_option

[WordPress Trac]

#63594: Bug/Broken State possible in Multisite Caching Code: update_network_option
--------------------------------+-----------------------------
 Reporter:  EliW                |      Owner:  (none)
     Type:  defect (bug)        |     Status:  new
 Priority:  normal              |  Milestone:  Awaiting Review
Component:  Options, Meta APIs  |    Version:  6.8
 Severity:  normal              |   Keywords:
  Focuses:                      |
--------------------------------+-----------------------------
 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:
 1. Have WordPress running with a Cache backend of your choice
 2. Create/Update a network option via update_network_option() or
 add_network_option()
 3. 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:
 {{{#!php
         $old_value = get_network_option( $network_id, $option );
 }}}

 And it will only 'add' the option, if it didn't exist already:
 {{{#!php
         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:

 {{{#!php

         $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:
 {{{#!php
                 if ( $result ) {
                         $cache_key = "$network_id:$option";
                         wp_cache_set( $cache_key, $value, 'site-options'
 );
                 }
 }}}

 To this:
 {{{#!php
                 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

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/63594>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform