WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 3 days ago

#33227 reviewing defect (bug)

"Delete Permantently" Only Deletes Original Size (UTF-8 file names)

Reported by: pavelevap Owned by: SergeyBiryukov
Milestone: Future Release Priority: normal
Severity: normal Version: 4.1
Component: Media Keywords: has-patch
Focuses: Cc:

Description

  • Use any standard image (for example 1024 x 768 px).
  • Name it Example.jpg
  • Upload image
  • Delete image
  • Check FTP, all image sizes deleted.

Now you can rename this file as Žlutý kůň.jpg, upload it and then delete. Only original full size was deleted, all other generated sizes are preserved on server. I found this problem when examined website with many forgotten images, which were not deleted as expected...

Attachments (1)

33227.patch (4.0 KB) - added by gitlost 4 months ago.
Fix for wp_delete_attachment() (only), includes unit test.

Download all attachments as: .zip

Change History (21)

#1 @ocean90
22 months ago

  • Component changed from General to Media
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version trunk deleted

Reproduced in trunk and 4.1.

This ticket was mentioned in Slack in #core by obenland. View the logs.


22 months ago

#3 @Pamela1991
17 months ago

This is actually not a problem with diacritics; rather, it is an issue with UTF-8 file names.

#4 @swissspidy
17 months ago

#35485 was marked as a duplicate.

#5 @Pamela1991
17 months ago

  • Summary changed from Image sizes with diacritic in filename not deleted to "Delete Permantently" Only Deletes Original Size (UTF-8 file names)
  • Version set to 4.4.1

This is actually not a problem with diacritics; rather, it is an issue with UTF-8 file names.

When deleting uploaded media files with UTF-8 file names, upon permanently deleting these files, only the original full size image is deleted, whereas all re-sized variants are not deleted.

Steps to reproduce this issue:

  1. Upload New Media file titled صورة.jpg via http://domain.com/wp-admin/media-new.php
  2. Delete Permanently from Media Library
  3. Navigate to FTP directory and only the original full-size picture will have been deleted. All re-sized versions remain.

I tried modifying wp_delete_attachment() in post.php by changing the following lines to no avail:

<?php
4703     $meta = utf8_encode(wp_get_attachment_metadata( $post_id ));
4704     $backup_sizes = utf8_encode(get_post_meta( $post->ID, '_wp_attachment_backup_sizes', true ));
4752     $thumbfile = mb_ereg_replace(basename($file), $meta['thumb'], $file);
4762     $intermediate_file = mb_ereg_replace( basename( $file ), $sizeinfo['file'], $file );

I'm not sure what else to try, but if someone can point me in the right direction as to what files or functions I need to play with, that would be excellent.

#6 follow-ups: @johnbillion
15 months ago

  • Keywords reporter-feedback added
  • Version changed from 4.4.1 to 4.1

I'm unable to reproduce this issue so far, even if I go back to 4.1. Resized image files with صورة or Žlutý-kůň in the name are all successfully deleted when the attachment is deleted. I've tested this with Linux and OS X environments.

@Pamela1991 @pavelevap @ocean90 Which operating system and PHP version is in use on your site?

#7 in reply to: ↑ 6 @Pamela1991
15 months ago

Replying to johnbillion:

I'm unable to reproduce this issue so far, even if I go back to 4.1. Resized image files with صورة or Žlutý-kůň in the name are all successfully deleted when the attachment is deleted. I've tested this with Linux and OS X environments.

@Pamela1991 @pavelevap @ocean90 Which operating system and PHP version is in use on your site?

Thanks. I'm using Ubuntu 14.04.4 LTS, Nginx 1.8.1, and PHP 5.6.18-1 and still experiencing this issue.

Last edited 15 months ago by Pamela1991 (previous) (diff)

#8 follow-up: @extendwings
15 months ago

@Pamela1991 @pavelevap Could you test WP Multibyte Patch? This plugin is included in Japanese core package by default.

#9 in reply to: ↑ 8 @Pamela1991
15 months ago

Replying to extendwings:

@Pamela1991 @pavelevap Could you test WP Multibyte Patch? This plugin is included in Japanese core package by default.

Thanks. I just tested this plugin. Upon uploading صورة.jpg, it is now uploaded as 22d882505c92fad3fcca446250f937e5.jpg (i.e. the actual file name is changed). In turn, deleting this newly uploaded image deletes the original as well as all other resized dimensions. However, this does not work for images that have already been uploaded with their UTF-8 file names. Moreover, I'd prefer to keep the UTF-8 file names instead of these non-pretty file names.

#10 in reply to: ↑ 6 @thomaswm
13 months ago

I can confirm this on Debian 8, PHP 5.6.20, WordPress 4.5.2 (multisite).

  • Uploaded image file äöü.jpg
  • Deleted the image in the media library
  • Intermediate sizes remain on server:
-rw-r--r-- 1 www-data www-data 158803 May  7 03:59 äöü-1024x768.jpg
-rw-r--r-- 1 www-data www-data   7052 May  7 03:59 äöü-150x150.jpg
-rw-r--r-- 1 www-data www-data  18258 May  7 03:59 äöü-300x225.jpg
-rw-r--r-- 1 www-data www-data  92252 May  7 03:59 äöü-768x576.jpg
-rw-r--r-- 1 www-data www-data  91580 May  7 03:59 äöü-825x510.jpg

Last edited 13 months ago by thomaswm (previous) (diff)

#11 @thomaswm
13 months ago

In order to track down the error, I've removed the @ in line 4917 of wp-includes/post.php:

            @ unlink( path_join( $uploadpath['basedir'], $intermediate_file ) );

When deleting the file äöü.jpg, I end up with the following lines in my error.log:

[Wed May 11 03:48:17.775811 2016] [:error] [pid 31260] [client 127.0.0.1:51648] PHP Warning:  unlink(/data/wordpress/www/wp-content/blogs.dir/169/files/2016/05/\xc3\xa4\xc3\xb6\xc3\xbc\xc3\xa4\xc3\xb6\xc3\xbc-150x150.jpg): No such file or directory in /data/wordpress/www/wp-includes/post.php on line 4917
[Wed May 11 03:48:17.775872 2016] [:error] [pid 31260] [client 127.0.0.1:51648] PHP Warning:  unlink(/data/wordpress/www/wp-content/blogs.dir/169/files/2016/05/\xc3\xa4\xc3\xb6\xc3\xbc\xc3\xa4\xc3\xb6\xc3\xbc-300x225.jpg): No such file or directory in /data/wordpress/www/wp-includes/post.php on line 4917
[Wed May 11 03:48:17.775903 2016] [:error] [pid 31260] [client 127.0.0.1:51648] PHP Warning:  unlink(/data/wordpress/www/wp-content/blogs.dir/169/files/2016/05/\xc3\xa4\xc3\xb6\xc3\xbc\xc3\xa4\xc3\xb6\xc3\xbc-768x576.jpg): No such file or directory in /data/wordpress/www/wp-includes/post.php on line 4917
[Wed May 11 03:48:17.775930 2016] [:error] [pid 31260] [client 127.0.0.1:51648] PHP Warning:  unlink(/data/wordpress/www/wp-content/blogs.dir/169/files/2016/05/\xc3\xa4\xc3\xb6\xc3\xbc\xc3\xa4\xc3\xb6\xc3\xbc-1024x768.jpg): No such file or directory in /data/wordpress/www/wp-includes/post.php on line 4917
[Wed May 11 03:48:17.775977 2016] [:error] [pid 31260] [client 127.0.0.1:51648] PHP Warning:  unlink(/data/wordpress/www/wp-content/blogs.dir/169/files/2016/05/\xc3\xa4\xc3\xb6\xc3\xbc\xc3\xa4\xc3\xb6\xc3\xbc-825x510.jpg): No such file or directory in /data/wordpress/www/wp-includes/post.php on line 4917

#12 @gitlost
4 months ago

As also reported in #39039, this is the same locale-dependent basename() bug as #21217 and #23267 and various others. The following patch fixes it for this instance, but from what I can see there's around 90 other instances of basename() and PATHINFO_BASENAME throughout the code base which should also use wp_basename() instead, so perhaps a new general "audit" ticket should be raised for them?

The patch changes basename() to wp_basename() in wp_delete_attachment() only, including the $meta['thumb'] branch use, though not sure how it arises or how to test it. The patch also fixes the Tests_Post_Attachments::test_insert_image_delete() test which wasn't checking the non-existence of deleted files correctly.

@gitlost
4 months ago

Fix for wp_delete_attachment() (only), includes unit test.

#13 @SergeyBiryukov
4 months ago

#39039 was marked as a duplicate.

#14 @SergeyBiryukov
4 months ago

  • Keywords has-patch added; needs-patch reporter-feedback removed
  • Milestone changed from Future Release to 4.8

#15 @SergeyBiryukov
4 months ago

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

#16 @gitlost
3 months ago

Note this could be by-passed by #39476.

This ticket was mentioned in Slack in #core by flixos90. View the logs.


8 days ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


8 days ago

This ticket was mentioned in Slack in #core by obenland. View the logs.


3 days ago

#20 @obenland
3 days ago

  • Milestone changed from 4.8 to Future Release
Note: See TracTickets for help on using tickets.