Opened 4 months ago
Last modified 9 days ago
#61117 new defect (bug)
Guard condition in `delete_temp_backup()` shouldn't return early
Reported by: | johnbillion | Owned by: | |
---|---|---|---|
Milestone: | 6.7 | Priority: | low |
Severity: | normal | Version: | 6.3 |
Component: | Upgrade/Install | Keywords: | has-patch needs-testing |
Focuses: | Cc: |
Description
In the delete_temp_backup()
function there is a guard condition which triggers an early return if the arguments for any of the temp backups are empty (ref).
This prevents further iteration of the array of temp backups. In theory this prevents the deletion of a valid backup that exists in in a subsequent element in the array.
I think the return false
should be replaced with continue
.
Attachments (1)
Change History (8)
#2
@
4 months ago
- Keywords has-patch added; needs-patch removed
- Milestone changed from Future Release to 6.6
This ticket was mentioned in Slack in #core by oglekler. View the logs.
3 months ago
#4
@
3 months ago
During today's bug scrub, it was highlighted that the patch needs testing, and guidance was sought for from people who know more about this. The discussion focused on whether true or false should be returned when deletion is skipped, considering the potential impact on other parts of the code. It was noted that low-level APIs typically return true for successful deletion or when the item to be deleted does not exist.
Suggestions for handling broken backups included using unset()
, adding an error, or utilizing wp_trigger_error()
behind a WP_DEBUG
flag.
We felt like a more extensive discussion is needed.
This ticket was mentioned in Slack in #core by nhrrob. View the logs.
3 months ago
#6
@
3 months ago
- Keywords needs-testing added
- Milestone changed from 6.6 to 6.7
We are very close to 6.6 RC1.
Punting to the next milestone.
Patch file