diff --git a/src/wp-includes/update.php b/src/wp-includes/update.php index 9f4257ab03da6..b729c268d0b40 100644 --- a/src/wp-includes/update.php +++ b/src/wp-includes/update.php @@ -316,7 +316,16 @@ function ( $carry, $prefixed_myisam_table ) use ( $all_unprefixed_tables ) { $updates->translations = $body['translations']; } - set_site_transient( 'update_core', $updates ); + if ( false === set_site_transient( 'update_core', $updates ) + && _wp_maybe_reset_update_check_lock( 'update_core', $updates, $current ) + ) { + wp_trigger_error( + __FUNCTION__, + 'The result of the WordPress version check could not be stored. ' . + 'The check will run again on the next request.', + E_USER_WARNING + ); + } if ( ! empty( $body['ttl'] ) ) { $ttl = (int) $body['ttl']; @@ -606,7 +615,16 @@ function wp_update_plugins( $extra_stats = array() ) { array_walk( $updates->response, $sanitize_plugin_update_payload ); array_walk( $updates->no_update, $sanitize_plugin_update_payload ); - set_site_transient( 'update_plugins', $updates ); + if ( false === set_site_transient( 'update_plugins', $updates ) + && _wp_maybe_reset_update_check_lock( 'update_plugins', $updates, $current ) + ) { + wp_trigger_error( + __FUNCTION__, + 'The result of the plugin update check could not be stored. ' . + 'The check will run again on the next request.', + E_USER_WARNING + ); + } } /** @@ -872,7 +890,56 @@ function wp_update_themes( $extra_stats = array() ) { } } - set_site_transient( 'update_themes', $new_update ); + if ( false === set_site_transient( 'update_themes', $new_update ) + && _wp_maybe_reset_update_check_lock( 'update_themes', $new_update, $last_update ) + ) { + wp_trigger_error( + __FUNCTION__, + 'The result of the theme update check could not be stored. ' . + 'The check will run again on the next request.', + E_USER_WARNING + ); + } +} + +/** + * Rolls back an update check lock when the result of the check could not be stored. + * + * wp_version_check(), wp_update_plugins(), and wp_update_themes() record a `last_checked` + * timestamp before contacting the API, so that concurrent requests do not all issue the + * same remote request. When the write that stores the result then fails, for example + * because the response contains characters the `wp_options` column cannot hold, that + * timestamp is left behind on stale data. The next check sees a recent `last_checked`, + * returns early, and once the timeout expires it fails and re-arms the lock the same way, + * so the site never sees another update. Resetting `last_checked` lets the following + * request retry the check instead. + * + * A false return from set_site_transient() does not on its own mean the write failed: + * update_site_option() also returns false when the value it is asked to store is already + * the stored value. That happens routinely here, because the result of a check that found + * nothing new is identical to the object stored when the lock was taken, down to the + * timestamp, whenever the API responds within the same second. The two are therefore + * compared before anything is rolled back, so an unchanged value is not mistaken for a + * failure and does not send the site back out to the API on every request. + * + * @since 7.1.0 + * @access private + * + * @param string $transient Update transient name. Accepts 'update_core', 'update_plugins', + * or 'update_themes'. + * @param mixed $value The value that set_site_transient() was asked to store. + * @param stdClass $lock The object that was stored when the lock was taken. + * @return bool Whether the lock was rolled back. + */ +function _wp_maybe_reset_update_check_lock( $transient, $value, $lock ) { + if ( maybe_serialize( $value ) === maybe_serialize( $lock ) ) { + return false; + } + + $lock->last_checked = 0; + set_site_transient( $transient, $lock ); + + return true; } /** diff --git a/tests/phpunit/tests/update/wpUpdatePlugins.php b/tests/phpunit/tests/update/wpUpdatePlugins.php new file mode 100644 index 0000000000000..c000fa4fadc1b --- /dev/null +++ b/tests/phpunit/tests/update/wpUpdatePlugins.php @@ -0,0 +1,270 @@ +request_count = 0; + + // Start from a clean slate so the first write creates the option rather than updating it. + delete_site_transient( 'update_plugins' ); + + add_filter( 'pre_http_request', array( $this, 'mock_update_check_response' ), 10, 3 ); + } + + public function tear_down() { + delete_site_transient( 'update_plugins' ); + + parent::tear_down(); + } + + /** + * Answers the update check without contacting api.wordpress.org, and counts the calls. + * + * @param false|array|WP_Error $response A preemptive return value of an HTTP request. + * @param array $args HTTP request arguments. + * @param string $url The request URL. + * @return array|false A mocked response for the plugin update check, false otherwise. + */ + public function mock_update_check_response( $response, $args, $url ) { + if ( false === strpos( $url, '://api.wordpress.org/plugins/update-check/' ) ) { + return $response; + } + + ++$this->request_count; + + return array( + 'headers' => array(), + 'body' => wp_json_encode( + array( + 'plugins' => array( + 'hello.php' => array( + 'slug' => 'hello-dolly', + 'plugin' => 'hello.php', + 'new_version' => '99.0', + 'url' => 'https://wordpress.org/plugins/hello-dolly/', + 'package' => 'https://downloads.wordpress.org/plugin/hello-dolly.99.0.zip', + ), + ), + 'translations' => array(), + 'no_update' => array(), + ) + ), + 'response' => array( + 'code' => 200, + 'message' => 'OK', + ), + 'cookies' => array(), + 'filename' => null, + ); + } + + /** + * Rejects the write that stores the result of the check, leaving the stored value untouched. + * + * This reproduces a database that cannot store the payload, for instance a `wp_options` + * column using utf8mb3 and an update response containing a four byte character. Only the + * result payload is rejected: it is the one carrying a `checked` property, while the object + * written when the lock is taken, and the rolled back copy of it, are not. + * + * @param mixed $value The new option value. + * @param mixed $old_value The old option value. + * @return mixed The value to store. + */ + public function reject_update_result( $value, $old_value ) { + if ( is_object( $value ) && isset( $value->checked ) ) { + return $old_value; + } + + return $value; + } + + /** + * The happy path must be untouched: the result is stored and the lock is left armed. + * + * @ticket 64550 + * + * @covers ::wp_update_plugins + */ + public function test_successful_check_stores_result_and_keeps_lock() { + wp_update_plugins(); + + $transient = get_site_transient( 'update_plugins' ); + + $this->assertIsObject( $transient, 'The update transient was not stored.' ); + $this->assertNotEquals( 0, $transient->last_checked, 'last_checked was reset after a successful check.' ); + $this->assertArrayHasKey( + 'hello.php', + $transient->response, + 'The update response was not stored.' + ); + $this->assertSame( 1, $this->request_count, 'The update check did not run exactly once.' ); + } + + /** + * A successful check must not send the site back to the API on the next request. + * + * @ticket 64550 + * + * @covers ::wp_update_plugins + */ + public function test_successful_check_is_not_repeated() { + wp_update_plugins(); + wp_update_plugins(); + + $this->assertSame( 1, $this->request_count, 'The update check ran again after it had succeeded.' ); + } + + /** + * When the result cannot be stored, the lock must not be left armed on stale data. + * + * Without this, `last_checked` stays at the time of a check whose result was never saved, + * so the site reports itself up to date until the timeout expires. + * + * @ticket 64550 + * + * @covers ::wp_update_plugins + */ + public function test_failed_write_resets_the_lock() { + if ( wp_using_ext_object_cache() ) { + $this->markTestSkipped( 'This test requires that an external object cache is not in use.' ); + } + + add_filter( 'pre_update_site_option__site_transient_update_plugins', array( $this, 'reject_update_result' ), 10, 2 ); + + // Note: $this->expectWarning() is deprecated and will be removed in PHPUnit 10. + $warnings = array(); + set_error_handler( + static function ( int $errno, string $errstr ) use ( &$warnings ) { + $warnings[] = compact( 'errno', 'errstr' ); + return true; + }, + E_USER_WARNING + ); + + wp_update_plugins(); + + restore_error_handler(); + + remove_filter( 'pre_update_site_option__site_transient_update_plugins', array( $this, 'reject_update_result' ), 10 ); + + $this->assertCount( 1, $warnings, 'A warning should be triggered when the result could not be stored.' ); + + $transient = get_site_transient( 'update_plugins' ); + + $this->assertIsObject( $transient, 'The update transient was not stored.' ); + $this->assertSame( + 0, + $transient->last_checked, + 'last_checked was left armed after the result could not be stored.' + ); + } + + /** + * After a failed write the next request must actually run the check again. + * + * @ticket 64550 + * + * @covers ::wp_update_plugins + */ + public function test_failed_write_allows_the_next_check_to_run() { + if ( wp_using_ext_object_cache() ) { + $this->markTestSkipped( 'This test requires that an external object cache is not in use.' ); + } + + add_filter( 'pre_update_site_option__site_transient_update_plugins', array( $this, 'reject_update_result' ), 10, 2 ); + + // Note: $this->expectWarning() is deprecated and will be removed in PHPUnit 10. + $warnings = array(); + set_error_handler( + static function ( int $errno, string $errstr ) use ( &$warnings ) { + $warnings[] = compact( 'errno', 'errstr' ); + return true; + }, + E_USER_WARNING + ); + + wp_update_plugins(); + + restore_error_handler(); + + remove_filter( 'pre_update_site_option__site_transient_update_plugins', array( $this, 'reject_update_result' ), 10 ); + + $this->assertCount( 1, $warnings, 'A warning should be triggered when the result could not be stored.' ); + + // The second check is the one that would never happen while the lock stayed armed. + wp_update_plugins(); + + $this->assertSame( 2, $this->request_count, 'The update check did not run again after a failed write.' ); + + $transient = get_site_transient( 'update_plugins' ); + + $this->assertArrayHasKey( + 'hello.php', + $transient->response, + 'The update response was not stored once the write succeeded again.' + ); + } + + /** + * An unchanged value must not be mistaken for a failed write. + * + * update_site_option() also returns false when the value it is asked to store is already + * the stored value, which happens whenever a check that found nothing new completes within + * the same second the lock was taken. Rolling the lock back in that case would send the + * site out to the API on every request. + * + * @ticket 64550 + * + * @covers ::_wp_maybe_reset_update_check_lock + */ + public function test_unchanged_value_does_not_reset_the_lock() { + $lock = (object) array( 'last_checked' => 1234567890 ); + $value = (object) array( 'last_checked' => 1234567890 ); + + $this->assertFalse( + _wp_maybe_reset_update_check_lock( 'update_plugins', $value, $lock ), + 'An unchanged value was treated as a failed write.' + ); + $this->assertSame( 1234567890, $lock->last_checked, 'last_checked was reset for an unchanged value.' ); + } + + /** + * A value that differs from the lock must roll the lock back. + * + * @ticket 64550 + * + * @covers ::_wp_maybe_reset_update_check_lock + */ + public function test_changed_value_resets_the_lock() { + $lock = (object) array( 'last_checked' => 1234567890 ); + $value = (object) array( + 'last_checked' => 1234567890, + 'checked' => array( 'hello.php' => '1.0' ), + ); + + $this->assertTrue( + _wp_maybe_reset_update_check_lock( 'update_plugins', $value, $lock ), + 'A failed write did not roll the lock back.' + ); + $this->assertSame( 0, $lock->last_checked, 'last_checked was not reset after a failed write.' ); + $this->assertSame( + 0, + get_site_transient( 'update_plugins' )->last_checked, + 'The rolled back lock was not stored.' + ); + } +}