Changeset 56796

Timestamp:

10/06/2023 04:31:22 PM (14 months ago)

Author:

flixos90

Message:

Options, Meta APIs: Fix bug with update_option() updating the wrong cache, leading to potentially stale values being returned.

When using the $autoload parameter of update_option() alongside an option value update, prior to this changeset the function would update the incorrect cache, not respecting the new autoload value. This could have severe implications such as returning a stale option value when the option in fact had already been deleted.

This changeset fixes the bug alongside test coverage that failed with trunk but now passes.

Props kkmuffme, pentatonicfunk, SergeyBiryukov, oglekler, azaozz, spacedmonkey, nicolefurlan, joemcgill, flixos90.
Fixes #51352.

Location:

trunk

Files:

• src/wp-includes/option.php (modified) (1 diff)
• tests/phpunit/tests/option/option.php (modified) (1 diff)

trunk/src/wp-includes/option.php

@@ -860 +860 @@
 
     if ( ! wp_installing() ) {
-        $alloptions = wp_load_alloptions( true );
-        if ( isset( $alloptions[ $option ] ) ) {
+        if ( ! isset( $update_args['autoload'] ) ) {
+            // Update the cached value based on where it is currently cached.
+            $alloptions = wp_load_alloptions( true );
+            if ( isset( $alloptions[ $option ] ) ) {
+                $alloptions[ $option ] = $serialized_value;
+                wp_cache_set( 'alloptions', $alloptions, 'options' );
+            } else {
+                wp_cache_set( $option, $serialized_value, 'options' );
+            }
+        } elseif ( 'yes' === $update_args['autoload'] ) {
+            // Delete the individual cache, then set in alloptions cache.
+            wp_cache_delete( $option, 'options' );
+
+            $alloptions = wp_load_alloptions( true );
             $alloptions[ $option ] = $serialized_value;
             wp_cache_set( 'alloptions', $alloptions, 'options' );
         } else {
+            // Delete the alloptions cache, then set the individual cache.
+            $alloptions = wp_load_alloptions( true );
+            if ( isset( $alloptions[ $option ] ) ) {
+                unset( $alloptions[ $option ] );
+                wp_cache_set( 'alloptions', $alloptions, 'options' );
+            }
+
             wp_cache_set( $option, $serialized_value, 'options' );
         }

trunk/tests/phpunit/tests/option/option.php

@@ -778 +778 @@
         $this->assertSame( 10, has_filter( 'pre_option_foo', '__return_zero' ) );
     }
+
+    /**
+     * Tests that calling update_option() with changed autoload from 'no' to 'yes' updates the cache correctly.
+     *
+     * This ensures that no stale data is served in case the option is deleted after.
+     *
+     * @ticket 51352
+     *
+     * @covers ::update_option
+     */
+    public function test_update_option_with_autoload_change_no_to_yes() {
+        add_option( 'foo', 'value1', '', 'no' );
+        update_option( 'foo', 'value2', 'yes' );
+        delete_option( 'foo' );
+        $this->assertFalse( get_option( 'foo' ) );
+    }
+
+    /**
+     * Tests that calling update_option() with changed autoload from 'yes' to 'no' updates the cache correctly.
+     *
+     * This ensures that no stale data is served in case the option is deleted after.
+     *
+     * @ticket 51352
+     *
+     * @covers ::update_option
+     */
+    public function test_update_option_with_autoload_change_yes_to_no() {
+        add_option( 'foo', 'value1', '', 'yes' );
+        update_option( 'foo', 'value2', 'no' );
+        delete_option( 'foo' );
+        $this->assertFalse( get_option( 'foo' ) );
+    }
 }