Skip to content
Open
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/wp-includes/cron.php
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ function wp_reschedule_event( $timestamp, $recurrence, $hook, $args = array(), $
}

// Now we try to get it from the saved interval in case the schedule disappears.
if ( 0 === $interval ) {
if ( 0 === (int) $interval ) {
Comment thread
westonruter marked this conversation as resolved.
Outdated
$scheduled_event = wp_get_scheduled_event( $hook, $args, $timestamp );

if ( $scheduled_event && isset( $scheduled_event->interval ) ) {
Expand Down Expand Up @@ -417,11 +417,15 @@ function wp_reschedule_event( $timestamp, $recurrence, $hook, $args = array(), $
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

if ( $wp_error ) {
return new WP_Error(
'invalid_schedule',
__( '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' ),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the $recurrence should be included here as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// 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;
}
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not like 063c21e?

wp_json_encode( $interval )
)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about c6efb67?

);
}

Expand Down
Loading