[wp-trac] [WordPress Trac] #44118: WordPress performs some unnecessary plugin update checks

[WordPress Trac]

#44118: WordPress performs some unnecessary plugin update checks
-------------------------------------+--------------------------
 Reporter:  siliconforks             |       Owner:  johnbillion
     Type:  defect (bug)             |      Status:  reviewing
 Priority:  normal                   |   Milestone:  6.8
Component:  Upgrade/Install          |     Version:
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |     Focuses:
-------------------------------------+--------------------------

Comment (by siliconforks):

 Replying to [comment:14 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.

 > * [attachment:"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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/44118#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform