Handle bad interval for cron schedule by westonruter · Pull Request #10619 · WordPress/wordpress-develop

westonruter · 2025-12-11T22:59:37Z

Trac ticket: https://core.trac.wordpress.org/ticket/64404

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2025-12-11T23:14:31Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

github-actions · 2025-12-14T22:29:54Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props westonruter, peterwilsoncc, johnbillion.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

…into trac-64404

westonruter · 2025-12-14T22:33:32Z

-				__( 'Event schedule does not exist.' )
+				sprintf(
+					/* translators: %s is the interval encoded as JSON */
+					__( 'Event schedule is invalid. Interval must be positive integer, but got: %s' ),


Maybe the $recurrence should be included here as well?

I think this would be good if a named schedule is used, ie if the code doesn't fall back to

wordpress-develop/src/wp-includes/cron.php

Lines 361 to 368 in 8b0250c

// Now we try to get it from the saved interval in case the schedule disappears.

if ( 0 === $interval ) {

$scheduled_event = wp_get_scheduled_event( $hook, $args, $timestamp );

if ( $scheduled_event && isset( $scheduled_event->interval ) ) {

$interval = $scheduled_event->interval;

}

}

Not like 063c21e?

peterwilsoncc

I've added a few notes inline.

peterwilsoncc · 2025-12-15T22:06:11Z


 	// Now we assume something is wrong and fail to schedule.
-	if ( 0 === $interval ) {
+	if ( ! is_int( $interval ) || $interval <= 0 ) {


Suggested change

if ( ! is_int( $interval ) || $interval <= 0 ) {

if ( ! is_numeric( $interval ) || $interval <= 0 ) {

Matches the behaivour of the existing check in wp_schedule_event()

peterwilsoncc · 2025-12-15T22:09:06Z

-				__( 'Event schedule does not exist.' )
+				sprintf(
+					/* translators: %s is the interval encoded as JSON */
+					__( 'Event schedule is invalid. Interval must be positive integer, but got: %s' ),


I think this would be good if a named schedule is used, ie if the code doesn't fall back to

wordpress-develop/src/wp-includes/cron.php

Lines 361 to 368 in 8b0250c

// Now we try to get it from the saved interval in case the schedule disappears.

if ( 0 === $interval ) {

$scheduled_event = wp_get_scheduled_event( $hook, $args, $timestamp );

if ( $scheduled_event && isset( $scheduled_event->interval ) ) {

$interval = $scheduled_event->interval;

}

}

…into trac-64404

Co-authored-by: Peter Wilson <[email protected]>

johnbillion · 2025-12-19T00:18:14Z

There's a failing test:

1) Tests_Cron::test_invalid_recurrence_for_event_returns_error
sprintf(): Argument number must be greater than zero

/var/www/src/wp-includes/cron.php:428
/var/www/tests/phpunit/tests/cron.php:856
phpvfscomposer:///var/www/vendor/phpunit/phpunit/phpunit:106
/var/www/vendor/bin/phpunit:118

johnbillion · 2025-12-19T00:26:22Z

+					__( 'Event schedule with recurrence "%$1s" does not exist or has is invalid. Interval must be positive integer, but got: %$2s' ),
+					$recurrence,
+					wp_json_encode( $interval )
+				)


It took me a couple of reads to understand what this is trying to tell me. The interval is an internal implementation detail that's not used by anyone calling this function, so I'm not sure it's an actionable message.

Is it possible to reliably distinguish between a schedule that doesn't exist and a schedule that exists but has an invalid interval? If so, let's use two separate error messages, one for each case, then make each error message clearer.

How about c6efb67?

…into trac-64404

Co-authored-by: John Blackbourn <[email protected]>

… wp_reschedule_event() Co-authored-by: John Blackbourn <[email protected]>

Copilot

Pull request overview

This pull request adds validation to handle invalid intervals for cron schedules, addressing ticket 64404. When a cron schedule is registered with an invalid interval (non-numeric or ≤ 0), the scheduling functions now return an appropriate error instead of allowing invalid data.

Changes:

Adds interval validation in wp_schedule_event() and wp_reschedule_event() to check for non-numeric or non-positive values
Introduces a new 'invalid_interval' error code to distinguish interval problems from missing schedules
Adds comprehensive test coverage for negative interval scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/wp-includes/cron.php	Adds validation for invalid intervals in both wp_schedule_event() (lines 319-328) and wp_reschedule_event() (lines 458-474), with appropriate error messages distinguishing between missing schedules and invalid intervals
tests/phpunit/tests/cron.php	Adds test_invalid_schedule_interval_returns_error() test case with a schedule containing a negative interval, verifies both functions return the correct error code, and adds ticket reference to related test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <[email protected]>

rbcorrales

Tested the PR locally, and left a couple of inline comments on an edge case in the interval validation with a suggestion.

rbcorrales · 2026-03-19T10:41:16Z

 		return false;
 	}

+	if ( ! is_numeric( $event->interval ) || $event->interval <= 0 ) {


Fractional floats between 0 and 1 (like 0.5, 0.1, 0.99) pass this validation because is_numeric(0.5) is true and 0.5 > 0. But PHP's % operator casts floats to int, so 0.5 becomes 0 and the modulo in wp_reschedule_event() still throws DivisionByZeroError.

Casting to (int) here catches these values at validation time, the same way the arithmetic would truncate them:

Suggested change

if ( ! is_numeric( $event->interval ) || $event->interval <= 0 ) {

if ( ! is_numeric( $event->interval ) || (int) $event->interval <= 0 ) {

I verified it locally without the cast, wp_schedule_event( time(), 'half_second_schedule', ... ) succeeds and wp_reschedule_event() crashes. With the cast, both return invalid_interval correctly.

rbcorrales · 2026-03-19T10:41:37Z


 	// Now we assume something is wrong and fail to schedule.
-	if ( 0 === $interval ) {
+	if ( ! is_numeric( $interval ) || $interval <= 0 ) {


Same suggestion here:

Suggested change

if ( ! is_numeric( $interval ) || $interval <= 0 ) {

if ( ! is_numeric( $interval ) || (int) $interval <= 0 ) {

Handle bad interval for cron schedule

101f58f

Fix logic inversion

b39b67d

westonruter requested a review from peterwilsoncc December 14, 2025 22:29

westonruter marked this pull request as ready for review December 14, 2025 22:29

Merge branch 'trunk' of https://github.com/WordPress/wordpress-develop …

d023b9a

…into trac-64404

westonruter commented Dec 14, 2025

View reviewed changes

Comment thread src/wp-includes/cron.php Outdated

westonruter commented Dec 14, 2025

View reviewed changes

peterwilsoncc requested a review from johnbillion December 15, 2025 21:43

peterwilsoncc reviewed Dec 15, 2025

View reviewed changes

westonruter and others added 4 commits December 16, 2025 21:59

Merge branch 'trunk' of https://github.com/WordPress/wordpress-develop …

3bba3b9

…into trac-64404

Revert initial casting to int

76f3901

Co-authored-by: Peter Wilson <[email protected]>

Use is_numeric() instead of is_int()

0184a4d

Co-authored-by: Peter Wilson <[email protected]>

Add recurrence to the error message

063c21e

westonruter requested a review from peterwilsoncc December 17, 2025 06:06

johnbillion reviewed Dec 19, 2025

View reviewed changes

westonruter and others added 4 commits December 18, 2025 16:30

Merge branch 'trunk' of https://github.com/WordPress/wordpress-develop …

662315d

…into trac-64404

Fix failing test

cda5935

Co-authored-by: John Blackbourn <[email protected]>

Add interval check to wp_schedule_event() and differentiate errors in…

c6efb67

… wp_reschedule_event() Co-authored-by: John Blackbourn <[email protected]>

Merge branch 'trunk' into trac-64404

d1c3ba6

westonruter requested a review from Copilot February 24, 2026 17:09

Copilot started reviewing on behalf of westonruter February 24, 2026 17:09 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

Comment thread tests/phpunit/tests/cron.php

Add additional test cases

0fae040

Co-authored-by: Copilot <[email protected]>

rbcorrales reviewed Mar 19, 2026

View reviewed changes

	// Now we try to get it from the saved interval in case the schedule disappears.
	if ( 0 === $interval ) {
	$scheduled_event = wp_get_scheduled_event( $hook, $args, $timestamp );

	if ( $scheduled_event && isset( $scheduled_event->interval ) ) {
	$interval = $scheduled_event->interval;
	}
	}

	if ( ! is_int( $interval ) \|\| $interval <= 0 ) {
	if ( ! is_numeric( $interval ) \|\| $interval <= 0 ) {

	if ( ! is_numeric( $event->interval ) \|\| $event->interval <= 0 ) {
	if ( ! is_numeric( $event->interval ) \|\| (int) $event->interval <= 0 ) {

	if ( ! is_numeric( $interval ) \|\| $interval <= 0 ) {
	if ( ! is_numeric( $interval ) \|\| (int) $interval <= 0 ) {

Conversation

westonruter commented Dec 11, 2025

Uh oh!

github-actions Bot commented Dec 11, 2025

Test using WordPress Playground

Some things to be aware of

Uh oh!

github-actions Bot commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterwilsoncc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnbillion commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

rbcorrales left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions Bot commented Dec 14, 2025 •

edited

Loading

johnbillion commented Dec 19, 2025 •

edited

Loading