WordPress performs some unnecessary plugin update checks

[siliconforks]

In the function wp_update_plugins(), it seems clear that the intent is not to check for updates if there has already been an update check recently (e.g., on the wp-admin/plugins.php page, "recently" means within the last hour). However, in some cases it will check for updates anyway because of subtle issues in the implementation.

The easiest way to reproduce the issue is to install the ​Query Monitor plugin (to view HTTP requests) and then perform the following steps:

1. Visit the admin section "Updates" page (wp-admin/update-core.php). Look at Query Monitor's "HTTP API Calls" - you should see a call to ​https://api.wordpress.org/plugins/update-check/1.1/ to check for plugin updates. (You might also see additional HTTP API calls for theme and core update checks.) Note: if you don't see the plugin update check, it's possible that there was already a check for updates in the last minute - update-core.php is supposed to check for updates no more than once per minute. Try waiting a minute and reloading the page.

2. Once you have seen the plugin update check being made, visit the "Plugins" page (wp-admin/plugins.php). Look at "HTTP API Calls" - you will see another plugin update check being made. This doesn't make much sense, because WordPress just checked for plugin updates on the update-core.php page.

3. Note that the additional update check in only occurs once. Try reloading the plugins.php page and look at "HTTP API Calls" again - this time there will be no plugin update check (which is the expected behavior, since WordPress just checked for plugin updates).

Looking at the code for wp_update_plugins(), it seems that the update_plugins transient is sometimes stored with a checked property, sometimes not. This doesn't make sense to me - when the checked property is not stored in the transient, the next call to wp_update_plugins() will always check for updates regardless of how recently the last update check occurred.

[siliconforks]

proposed fix

[johnbillion]

Thanks for the report and the patch @siliconforks . Sorry that it's taken so long to get a response.

This patch looks good but it needs some testing and a sanity check by someone familiar with this component.

[bookdude13]

Hey all, thanks for the patience on this issue :)

I can confirm that this behavior still exists in trunk as of today. I was also able to get the second request simply refreshing the update page after seeing the first plugin update request. After the second request there were no more until the 1min timeout was reached, then this could be repeated.

The patch applied cleanly and solved the described bug (no more double requests). I am new to this component though, so a sanity check is still needed.

[pbiron]

refreshing patch

[pbiron]

44118.diff​ refreshes the patch so that it applies cleanly.

[francina]

I tested the patch today

• MacOS 11.4
• Safari 16611.2.7.1.4
• wordpress-develop
• WordPress 5.8-beta1-51132-src

I can reproduce the issues, but the patch doesn't apply. This is the result I get

Hunk #1 succeeded at 151 with fuzz 1 (offset -145 lines).
Hunk #2 FAILED at 182.
Hunk #3 succeeded at 424 with fuzz 1 (offset 8 lines).
1 out of 3 hunks FAILED -- saving rejects to file src/wp-includes/update.php.rej

And here is what I have in the file:

*** 179,184 ****
  		$plugin_changed = false;
  
  		foreach ( $plugins as $file => $p ) {
  			if ( ! isset( $current->checked[ $file ] ) || (string) $current->checked[ $file ] !== (string) $p['Version'] ) {
  				$plugin_changed = true;
  			}
--- 182,189 ----
  		$plugin_changed = false;
  
  		foreach ( $plugins as $file => $p ) {
+ 			$new_option->checked[ $file ] = $p['Version'];
+ 
  			if ( ! isset( $current->checked[ $file ] ) || (string) $current->checked[ $file ] !== (string) $p['Version'] ) {
  				$plugin_changed = true;
  			}

I think the patch needs a refresh :) Thanks!

[siliconforks]

Refreshed patch to work after r50921 (which added support for the Update URI header)

[siliconforks]

Refreshed patch to work after r54891 (which modified coding standards to require parentheses for calls to constructors)

[siliconforks]

The previous patch no longer applied cleanly due to a coding standards change - I've uploaded a refreshed version.

Also, as this bug report approaches its fifth anniversary, it seems that there's not a lot of interest in fixing this issue... I'd just like to point out that I don't think I'm the only WordPress user who would like to see a fix for this; there appears to be some broader concern in the WordPress community that WordPress is performing an excessive number of outgoing HTTP requests. This project aims to make a plugin which will reduce these:

​https://www.cloudfest.com/eco-mode-reduce-outgoing-network-traffic-of-your-wordpress-server

That plugin might indeed be useful; however, I think it would be even better to fix any issue in the WordPress core which may be causing it to perform unnecessary HTTP requests in the first place...

[desrosj]

Moving to 6.8 to try and resolve this with #61055.

[johnbillion]

There is a subtle difference between the patch on this ticket and the PR on #61055, and I think we should go with the latter.

• 44118-after-coding-standards-change.diff​ adds all plugins to the $updates->checked property, which isn't strictly accurate because plugins could have been added or manually updated in the interim.
• ​https://github.com/WordPress/wordpress-develop/pull/6736 copies the value from $current->checked to $updates->checked which means its value correctly represents the list of plugins that were checked.

@siliconforks @Cybr @snehapatil02 Does that sound correct to you?

[siliconforks]

Replying to johnbillion:

There is a subtle difference between the patch on this ticket and the PR on #61055, and I think we should go with the latter.

Yes, there is definitely a difference there, but isn't that a bug with ​https://github.com/WordPress/wordpress-develop/pull/6736 ?

Here is a scenario to illustrate:

1. Suppose that ​https://github.com/WordPress/wordpress-develop/pull/6736 is applied, and suppose that there is one plugin installed - classic-editor, version 1.6.7. Suppose that the update_plugins transient has a checked property which contains ['classic-editor/classic-editor.php' => '1.6.7']. (Nothing out of the ordinary so far.)

2. Install another plugin, the classic-widgets plugin, manually, by unpacking a .zip file. (No need to activate it.)

3. Wait at least one minute and visit /wp-admin/update-core.php. This will call wp_update_plugins(). The $time_not_changed variable will be false. This means that all the code in the if ( $time_not_changed && ! $extra_stats ) statement will not be executed. So that means it's not going to bail out - it's going to proceed with the API call to https://api.wordpress.org/plugins/update-check/1.1/ and it will submit the following data:

   ...
   'classic-editor/classic-editor.php' => [
     ...
     'Version' => '1.6.7',
     ...
   ],
   'classic-widgets/classic-widgets.php' => [
     ...
     'Version' => '0.3',
     ...
   ],
   ...
   

4. So far everything is working fine, but the value that will be saved in the update_plugins transient checked property is: ['classic-editor/classic-editor.php' => '1.6.7']. In other words, it's saving the same value that was there previously.

5. Now suppose that the next time wp_update_plugins() gets called, the timeout has not expired, and the $time_not_changed variable is set to true, and it will execute the code in the if statement. It will loop through all the installed plugins, eventually hitting the classic-widgets plugin. Because this is not in $current->checked, the $plugin_changed variable will be set to true, and it's going to proceed with the API call again. It will submit the following data:

   ...
   'classic-editor/classic-editor.php' => [
     ...
     'Version' => '1.6.7',
     ...
   ],
   'classic-widgets/classic-widgets.php' => [
     ...
     'Version' => '0.3',
     ...
   ],
   ...
   

But that's the same thing it submitted last time. It's making the same API call it did last time, and the timeout has not expired, so we're basically back to the same issue - WordPress is still performing some unnecessary plugin update checks. Not as many as before, but there are still situations like this where it's making the API call unnecessarily.

• 44118-after-coding-standards-change.diff​ adds all plugins to the $updates->checked property, which isn't strictly accurate because plugins could have been added or manually updated in the interim.

I don't really understand this. The patch is adding all plugins to the $updates->checked property, which will include any plugins which have been added or manually updated since the last update check. So this list of plugins should be up-to-date and accurate, and it should be the same as the list of plugins that was submitted in the API call. I think this is what we want? Can you provide a more detailed example of a scenario where this would cause problems?

• ​https://github.com/WordPress/wordpress-develop/pull/6736 copies the value from $current->checked to $updates->checked which means its value correctly represents the list of plugins that were checked.

The problem (as I've illustrated in the scenario above) is that $current->checked may potentially be out of date, and then it gets copied to $updates->checked, so it might potentially be out of date too. This gets saved to the transient, so it's potentially saving old, out-of-date data to the transient.

[johnbillion]

@siliconforks It looks you're right! Thanks a lot for the explanation. I'll carry on testing to confirm for sure, then hopefully we can get this in.

[siliconforks]

Replying to johnbillion:

@siliconforks It looks you're right! Thanks a lot for the explanation. I'll carry on testing to confirm for sure, then hopefully we can get this in.

Just a suggestion for you or anyone else who is reviewing/testing this: it may be helpful to compare the behavior of wp_update_plugins() to wp_update_themes(). The code for the two functions is very similar - or at least, it was very similar originally, though it's diverged a lot over the years.

The fundamental problem with wp_update_plugins() is that it's essentially trying to ​jam two different loops into one. Conceptually, there are two separate loops here: there's a loop which goes through all plugins and determines whether any have changed since the last update, and there's another loop which populates the checked array which ultimately gets saved in the transient. The issue with wp_update_plugins() is that it tries to do both these things in a single loop. That doesn't really work here, because the loop which populates the checked array should always be executed, while the loop that determines whether any plugins have changed is sometimes skipped over entirely.

In contrast, if you look at the code for wp_update_themes(), it doesn't have this issue. There are two separate loops, one which populates the checked array, and one which goes through all the themes to see whether any have changed.

My patch basically just attempts to split the code into two separate loops and make the behavior of wp_update_plugins() a little bit more like wp_update_themes().

[johnbillion]

Yeah I also noticed that those two functions have diverged. I don't know if it's worth spending time to bring them back inline.

I've been testing this change and I concur that the preferred behaviour is to always push the current full list of plugin data into the changed property otherwise it would go stale from one check to another. Populating this property is correctly preventing the unnecessary update check from being triggered on the Plugins screen when the plugin update transient is otherwise up to date and doesn't require a refresh.

[johnbillion]

In 59926:

Upgrade/Install: Prevent an unnecessary plugin update check when the plugin update data is up to date.

This ensures the checked property is always populated with the latest plugin data before determining whether to perform an update check. Previously this was only populated when the data was already identified as being stale, which could result in a subsequent unnecessary update check when viewing the Plugins screen.

Props siliconforks, bookdude13, pbiron, francina, Cybr, snehapatil02, johnbillion

Fixes #44118, #61055