Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#54906 closed defect (bug) (fixed)

Check _get_cron_array type in upgrade_590 routine

Reported by: chrisvanpatten's profile chrisvanpatten Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 5.9.1 Priority: normal
Severity: normal Version: 5.9
Component: Upgrade/Install Keywords: has-patch fixed-major dev-reviewed
Focuses: Cc:

Description

When the upgrade_590 DB routine is run on a site without the cron site option, _get_cron_array returns false which causes a TypeError when the return value is run through array_filter.

https://github.com/WordPress/WordPress/blob/638ef7815ec29318434365ffde1d3cef7437b948/wp-admin/includes/upgrade.php#L2275-L2277

[25-Jan-2022 21:33:45 UTC] PHP Fatal error:  Uncaught TypeError: array_filter(): Argument #1 ($array) must be of type array, bool given in /var/www/html/web/wp/wp-admin/includes/upgrade.php:2273

In particular, I encountered this on a local multisite network with several sites which had been created but not yet used. Running the upgrade routine worked on the sites which had been used (and thus had cron site options) but failed on the others.

Related: #53940

Attachments (1)

54906.diff (610 bytes) - added by kapilpaul 2 years ago.
Created patch

Download all attachments as: .zip

Change History (13)

#1 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 5.9.1

#2 @peterwilsoncc
2 years ago

  • Version set to 5.9

Thanks for logging this Chris, I'll look at getting a fix in for the 5.9.1 release.

This ticket was mentioned in PR #2251 on WordPress/wordpress-develop by kapilpaul.


2 years ago
#3

  • Keywords has-patch added

@kapilpaul
2 years ago

Created patch

#4 @peterwilsoncc
2 years ago

  • Keywords commit added
  • Owner set to peterwilsoncc
  • Status changed from new to assigned

Thanks for the patch @kapilpaul.

The code looks good to me so I've self assigned the ticket and marked it for commit.

#5 @peterwilsoncc
2 years ago

  • Component changed from Cron API to Upgrade/Install

#6 @peterwilsoncc
2 years ago

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

In 52656:

Upgrade: Prevent warnings upgrading cron array.

An unvisited site may have an undefined cron array, resulting in _get_cron_array() returning the value false. Previously this would trigger warning in upgrade_590() as the function assumed _get_cron_array() would alway return an array.

No database version change is required as the upgrade routine was successful on sites with a cron array during 5.9.0. On sites without a cron array, the error has already been thrown if they are running db version 51917. This fix is only required for new sites or those upgrading that have skipped 5.9.0.

Follow up to [51917].

Props chrisvanpatten, kapilpaul, SergeyBiryukov.
Fixes #54906.
See #53940.

#7 @peterwilsoncc
2 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for merging in to the 5.9 branch.

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


2 years ago

#10 @audrasjb
2 years ago

  • Keywords dev-reviewed added; commit removed

In the comment, it could be nice to also mention this ticket (#54906).

Otherwise, the logic in this patch looks good to backport. Adding dev-reviewed keyword.

#11 @peterwilsoncc
2 years ago

In 52662:

Upgrade: Add follow up ticket ID to upgrade_590() comment.

Follow up to [52656].

Props audrasjb.
See #54906.

#12 @peterwilsoncc
2 years ago

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

In 52663:

Upgrade: Prevent warnings upgrading cron array.

An unvisited site may have an undefined cron array, resulting in _get_cron_array() returning the value false. Previously this would trigger warning in upgrade_590() as the function assumed _get_cron_array() would alway return an array.

No database version change is required as the upgrade routine was successful on sites with a cron array during 5.9.0. On sites without a cron array, the error has already been thrown if they are running db version 51917. This fix is only required for new sites or those upgrading that have skipped 5.9.0.

Follow up to [51917].

Props chrisvanpatten, kapilpaul, SergeyBiryukov, audrasjb.
Merges [52656] and [52662] to the 5.9 branch.
Fixes #54906.
See #53940.

Note: See TracTickets for help on using tickets.