#50436 closed defect (bug) (fixed)
Unnecessary DISABLE_WP_CRON check in plugin and theme auto updates
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | 5.5 |
Component: | Security | Keywords: | has-patch |
Focuses: | Cc: |
Description
The wp_get_auto_update_message()
function added in [47835] checks the value of the DISABLE_WP_CRON
constant and returns an error if it's set to true
.
This check is unnecessary. The DISABLE_WP_CRON
constant disables the cron spawner, not cron altogether, and is used when an external cron spawner is in use (eg. a system-level cron runner). The use of this constant doesn't indicate a problem that needs to be shown here.
The check for this constant can be removed altogether.
Attachments (2)
Change History (13)
This ticket was mentioned in PR #343 on WordPress/wordpress-develop by helloarpitgshah.
5 years ago
#1
- Keywords has-patch added; needs-patch removed
#2
@
5 years ago
- Keywords dev-feedback added
CC @bookdude13, it looks like you introduced this check into the feature plugin. Any concerns here?
#3
@
5 years ago
- Keywords dev-feedback removed
- Owner set to audrasjb
- Status changed from new to accepted
Thanks for spotting this @johnbillion and for the complete explanation.
This was introduced a long time ago in the feature plugin, but indeed it is not needed.
The provided pull request looks good to me, I think we can move forward.
I will update the feature plugin accordingly.
#4
@
5 years ago
I made that check without fully understanding the constant, so if removing it seems good then do it :)
#5
@
5 years ago
As John says, that constant doesn't prevent a direct request of /wp-cron.php
from triggering WP-Cron jobs (e.g., from a UNIX cron job; nor them being manually triggered by something like WP Crontrol).
But I thought the reason for that check was to provide "context" for why the job may be overdue, but since it does an early when the constant is true the message could be misleading.
Let me work up a patch that moves the check within the block that executes only if the job is overdue...
#6
@
5 years ago
On 2nd thought, I now agree with @johnbillion. The check for DISABLE_WP_CRON
being true is unnecessary.
50436.diff refactors wp_get_auto_update_message()
so that:
- there is no check for
DISABLE_WP_CRON
- all returned strings now begin with
Automatic update...
(Automatic update not scheduled.
,Automatic update overdue by...
andAutomatic update scheduled in...
) - there is a single return
#7
@
5 years ago
- Keywords good-first-bug removed
Just noting we should replace all Automatic updates
instances with Auto-updates
for better consistency :)
Otherwise, patch looks good to me 👍
#8
@
5 years ago
50436.1.diff is the same as 50436.diff, but uses Auto-update...
to begin each string.
Trac ticket:
https://core.trac.wordpress.org/ticket/50436