wp_generate_attachment_metadata() can overwrite previously uploaded images with the intermediate sized images it creates

[pbiron]

When ​wp_generate_attachment_metadata() creates/saves intermediate sized images, it can sometimes overwrite an original image from a previous upload.

Steps to reproduce:

1. set thumbnail size to 150x150
2. create a 300x300 image named foo-150x150.jpg
3. upload foo-150x150.jpg
4. create another 300x300 image named foo.jpg
5. upload foo.jpg

If you then check the appropriate dir in /wp-content/uploads you will see that the original foo-150x150.jpg has been overwritten by the thumbnail-sized version of foo.jpg.

Original filenames containing -{width}x{height} suffixes can be quite common, especially on artwork-related sites. A real-world example of the "steps to reproduce" above is as follows:

1. a painter does an oil painting of a sunset on a 150cmx150cm canvas and uploads a photo of that artwork named sunset-150x150.jpg (where, in their mind, the -150x150 suffix is the canvas size of their painting)
2. a photographer then uploads their photograph of a sunset named sunset.jpg

...and the photographer has just overwritten the painter's original image :-(

[pbiron]

example image to use in the "steps to reproduce"

[pbiron]

example image to use in the "steps to reproduce"

[pbiron]

There are a number of places in the process where the intermediate sized filenames could be "unique"-ified, e.g.,

• WP_Image_Editor::generate_filename()
• WP_Image_Editor::get_output_format()
• WP_Image_Editor_GD::multi_resize() (and the equivalent in WP_Image_Editor_Imagick::multi_resize()).

I'm fairly unfamiliar with that part of the core code so I'm not really sure which of those place (and maybe others that I've discovered) would be the safest/most reliable for core's needs.

However, I will note that 2 very widely used plugins (​Regenerate Thumbnails and ​WooCommerce) make calls to WP_Image_Editor::generate_filename() but not WP_Image_Editor_{GD|Imagick}::multi_resize()when they resize images.

So, I'm very interested in the opinion of those who are familiar with both the relevant parts of the core code and how other plugins would deal with any change.

[swissspidy]

This is a known issue. If I could just find the original ticket right now...

[pbiron]

Replying to swissspidy:

This is a known issue. If I could just find the original ticket right now...

I looked to see if it was a known issue before opening this ticket and couldn't find anything.

[desrosj]

I think #42437 is the one. Feel free to reopen if this is something different.

[pbiron]

Replying to desrosj:

I think #42437 is the one. Feel free to reopen if this is something different.

yes, that ticket is the same problem. I'll join the discussion over there.