Make WordPress Core

Opened 6 weeks ago

Last modified 2 days ago

#61117 new defect (bug)

Guard condition in `delete_temp_backup()` shouldn't return early

Reported by: johnbillion's profile johnbillion Owned by:
Milestone: 6.6 Priority: low
Severity: normal Version: 6.3
Component: Upgrade/Install Keywords: has-patch
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)

patch.diff (480 bytes) - added by amitraj2203 5 weeks ago.
Patch file

Download all attachments as: .zip

Change History (5)

@amitraj2203
5 weeks ago

Patch file

#1 @amitraj2203
5 weeks ago

@johnbillion I have submitted the patch for the same.

#2 @SergeyBiryukov
5 weeks 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.


2 days ago

#4 @jamieblomerus
2 days 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.

Note: See TracTickets for help on using tickets.