Make WordPress Core

Opened 5 months ago

Last modified 3 weeks ago

#61525 new defect (bug)

Plugins page keeps making HTTP requests for plugin dependencies which are not in the WordPress Plugin Directory

Reported by: siliconforks's profile siliconforks Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.5
Component: Plugins Keywords: has-patch close
Focuses: Cc:

Description

Suppose you have two plugins installed, foo and bar, and foo is a dependency of bar.

<?php

/*
Plugin Name: Bar
Requires Plugins: foo
*/

Suppose also that foo and bar are not in the WordPress Plugin Directory - they are custom plugins or third-party plugins installed from elsewhere.

Then, every time the Plugins admin page is visited, WP_Plugin_Dependencies::get_dependency_api_data() will make an HTTP request for information on foo:

https://api.wordpress.org/plugins/info/1.2/
?action=plugin_information
&request%5Bslug%5D=foo
&request%5Bfields%5D%5Bshort_description%5D=1
&request%5Bfields%5D%5Bicons%5D=1
&request%5Blocale%5D=en_US
&request%5Bwp_version%5D=6.7

Normally, for a plugin which is found in the WordPress Plugin Directory, the response to this request would be cached in a transient for 12 hours, and there would not be any need to repeat the HTTP request for plugin information during that time. However, because foo is not in the Plugin Directory, the response will return 404 Not Found, and this means that the HTTP request will keep getting made over and over again every time the Plugins page is visited.

Some thoughts on how to improve this behavior:

  1. Maybe WP_Plugin_Dependencies::get_dependency_api_data() should cache negative responses? That is, maybe it should remember that the HTTP request returned 404 Not Found and not try the request again for 12 hours?
  1. Maybe it would be wise to look at the Update URI header field (if it exists) and, if the value is not https://wordpress.org/plugins/{$slug}/ or w.org/plugin/{$slug}, then clearly the plugin is not in the WordPress Plugin Directory and the HTTP request for plugin information can be skipped? This seems like it would work; on the other hand, this might be considered to be (ab)using the "Update URI" header for a purpose other than that for which it was intended.

Change History (11)

#1 follow-up: @the ank
4 months ago

Hi @siliconforks, I tried to replicate this issue in my local WP. I found it keeps triggering the plugin check API, but it only triggers on the wp-admin/plugins.php page and not on other pages.

So yes, if the admin visits the plugins page multiple times, it will trigger the same API for missing required plugins not listed in WordPress.org.

Solution - When the first time this plugin API triggers, it should keep a log of required plugins not in wordpress.org and ignore those plugins to trigger check API.

Code -

$information = plugins_api(
				'plugin_information',
				array(
					'slug'   => $slug,
					'fields' => array(
						'short_description' => true,
						'icons'             => true,
					),
				)
			);

Regards

Last edited 4 months ago by the ank (previous) (diff)

#2 in reply to: ↑ 1 @siliconforks
4 months ago

Replying to the ank:

Solution - When the first time this plugin API triggers, it should keep a log of required plugins not in wordpress.org and ignore those plugins to trigger check API.

Yes, that should basically work, but ideally it should also handle cases where developers are working on custom plugins on their own server, and then later they submit the plugins to the WordPress Plugin Directory.

Using the example above: foo might be a custom plugin, but some day in the future it might actually show up in the Plugin Directory, and at that point it would make sense to retrieve plugin information from the Plugin Directory with an HTTP request.

This ticket was mentioned in PR #6944 on WordPress/wordpress-develop by @the ank.


4 months ago
#3

  • Keywords has-patch added

Issue - The plugins page keeps making HTTP requests for plugin dependencies which are not in the WordPress Plugin Directory

Solution -

File - /wp-includes/class-wp-plugin-dependencies.php
Line - 677 to 683 and 719 to 722
comment - Added by AV start and Added by AV ends

Fix - Required plugins that are not listed in wordpress.org, will have their own transient set for 12 hours and the next time before plugins_api() runs, it will check if transient exists, ignore this plugin to check in wordpress.org

#4 @the ank
4 months ago

  • Keywords has-patch removed

I have created a pull request with a solution for this issue: https://github.com/WordPress/wordpress-develop/pull/6944

#5 @sabernhardt
4 months ago

  • Keywords has-patch added

#6 @hellofromTonya
2 months ago

  • Milestone changed from Awaiting Review to 6.7
  • Version changed from 6.6 to 6.5

Appears the issue was not introduced in 6.6, but rather during the 6.5 cycle. Changed the Version to 6.5.

Moving it into 6.7.

Pinging @costdev for awareness and his expertise.

@costdev commented on PR #6944:


2 months ago
#7

This should already be being handled by the existing transient, but it seems something possibly isn't working as expected. Pinging @afragen on this as I'll be unavailable for the next number of days.

#8 @afragen
2 months ago

Actually this shouldn't be handled by the existing transient as the example used by the OP is __doing_it_wrong. The transient was designed to store data from a successful call to the Plugins API.

Clearly when testing, by using a plugin slug that doesn't exist or doesn't yet exist, the expectation is that no useful data will be returned and the dependency will not be met. As the data returned isn't useful it's not stored. It's possible that the slug was simply entered with a typo or the Plugins API returned an error due to any number of potential wp_remote_get() related issues. When Plugins API issue resolves, that data will be properly stored. If we would have stored the initial incorrect data it might be 12 hours until actual data is available and the user might infer that something is broken.

If the slug is for a premium plugin or a plugin that simply isn't available in the Plugin Repository, we have written the Advanced Plugin Dependencies plugin to solve that issue. Before anyone asks, this plugin was not allowed in the Plugin Repository for fear some developer might misuse it. Nothing in the plugin is at issue only how it might be used.

The solution is to enter a proper slug as a dependency for a plugin in the Plugin Repository, or use the Advanced Plugin Dependencies plugin with data for a plugin not in the Plugin Repository.

This ticket was mentioned in Slack in #core by chaion07. View the logs.


3 weeks ago

#11 @desrosj
3 weeks ago

  • Keywords close added
  • Milestone changed from 6.7 to Awaiting Review

WordPress 6.7 RC1 is due out any moment. So I'm going to punt this one. Based on the latest feedback though, it seems this should be closed out as a wontfix.

Adding the close suggestion to allow a bit more time for feedback in case something is being missed.

Note: See TracTickets for help on using tickets.