WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#47185 closed defect (bug) (fixed)

Media files cannot be deleted under Windows since 5.2

Reported by: tonybogdanov Owned by: SergeyBiryukov
Milestone: 5.2.1 Priority: normal
Severity: critical Version: 5.2
Component: Filesystem API Keywords: has-patch commit
Focuses: Cc:

Description

The wp_delete_file_from_directory function was introduced in 4.9.7 and is used when deleting media files from the uploads directory. However, it's internals were changed in 5.2 and the function no longer works on Windows.

The actual deletion is omitted if this check is truthy:

https://core.trac.wordpress.org/browser/tags/5.2/src/wp-includes/functions.php#L6292

And on a Windows machine trailingslashit adds a forward slash / at the end of the path whereas the first part of the path is using backslashes (due to realpath), thus strpos fails.

Additionally returning FALSE from wp_delete_attachment_files doesn't seem to be accounted for (haven't dug into it). When I delete a file in the media panel it "succeeds" even though the physical file hasn't been deleted.

Attachments (1)

47185.diff (980 bytes) - added by SergeyBiryukov 4 months ago.

Download all attachments as: .zip

Change History (8)

#1 @SergeyBiryukov
4 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.2.1

#2 @SergeyBiryukov
4 months ago

Introduced in [45177].

#3 @joemcgill
4 months ago

Looks like this was just missed in testing. Short term would be to revert [45177] in the next minor unless someone has a patch to resolve this fully without reintroducing the issues from #44563 in the mean time.

#4 @SergeyBiryukov
4 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to accepted

#5 @SergeyBiryukov
4 months ago

  • Keywords has-patch commit added; needs-patch removed

The gist is that wp_normalize_path() needs to be applied both before and after realpath(), which may seem redundant at a glance, but was deemed necessary when testing the original [43392] changeset.

#6 @SergeyBiryukov
4 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 45352:

Media: Fix deletion of files on Windows.

wp_delete_file_from_directory() should always normalize file paths before comparing.

Props tonybogdanov, SergeyBiryukov.
Fixes #47185.

#7 @SergeyBiryukov
4 months ago

In 45353:

Media: Fix deletion of files on Windows.

wp_delete_file_from_directory() should always normalize file paths before comparing.

Props tonybogdanov, SergeyBiryukov.
Merges [45352] to the 5.2 branch.
Fixes #47185.

Note: See TracTickets for help on using tickets.