Make WordPress Core

Opened 2 months ago

Closed 7 weeks ago

Last modified 7 weeks ago

#60540 closed defect (bug) (fixed)

Plugin dependencies: guard against unexpected responses to the `plugin_information` API endpoint

Reported by: pbiron's profile pbiron Owned by: costdev's profile costdev
Milestone: 6.5 Priority: normal
Severity: normal Version: 6.5
Component: Plugins Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description (last modified by costdev)

WP_Plugin_Dependencies::get_dependency_api_data() calls the plugin_information endpoint of the Plugins API.

The existing code in 6.5 Beta 1 assumes that all responses that are not WP_Error instances are produced by the .org API. However, extenders are able to filter the responses and some premium plugins do that to provide info about the premium plugins. However, some of those extenders may return responses to that endpoint that do not contain properties that the Plugin Dependencies codebase relies on.

Thus, rather than just checking whether the response is a WP_Error, we need to check that all the properties in the response that are used are actually present in said response.

Change History (7)

#1 @costdev
2 months ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 6.5

#2 @pbiron
2 months ago

For instance, suppose a "site-specific" plugin adds a Required Plugins: gravityforms header. Gravity Forms is one such plugin that hooks in the Plugins API and returns a response to the plugin_information endpibnt.

However, it's response does not contain the slug property. The existing 6.5 Beta 1 code expects slug to be populated in the response...and a Undefined property: $slug PHP run-time error is generated in this case. While that is a bug in GF to not populate that field in the response, we should account for missing properties in the API responses.

Note: I realize that no plugin will be allowed in the .org repo that would have a Required Plugins header with slugs of plugins that are not in the .org repo, there is nothing stopping "site-specific" plugins from doing so. Hence, we should make sure that doing so does not cause run-time PHP errors.

#3 @costdev
2 months ago

  • Owner set to costdev
  • Status changed from new to assigned

This ticket was mentioned in PR #6151 on WordPress/wordpress-develop by @costdev.


2 months ago
#4

  • Keywords has-patch has-unit-tests added

Previously, WP_Plugin_Dependencies::get_dependency_api_data() attempted to set an array key using the slug property returned in a Plugins API response. However, the Plugins API response is filterable and may not contain a slug property.

Earlier in the method, a local $slug variable is used as an array key.

For safety and consistency, this replaces references to $information->slug with $slug.

#5 @costdev
7 weeks ago

  • Keywords commit added

PR 6151 has approval. I've rebased it on trunk. Waiting for CI to pass then committing.

#6 @costdev
7 weeks ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 57736:

Plugin Dependencies: Don't assume API response has a slug property.

Previously, WP_Plugin_Dependencies::get_dependency_api_data() attempted to set an array key using the slug property returned in a Plugins API response. However, the Plugins API response is filterable and may not contain a slug property.

Earlier in the method, a local $slug variable is used as a key for the same array.

For safety and consistency, this replaces array key references to $information->slug with $slug.

Follow-up to [57545].

Props pbiron, afragen, swissspidy, costdev.
Fixes #60540.

@costdev commented on PR #6151:


7 weeks ago
#7

Committed in r57736.

Note: See TracTickets for help on using tickets.