[wp-trac] [WordPress Trac] #53668: Generated images for one file can be overwritten by another with the same name when mapping mime types for generated images

[WordPress Trac]

#53668: Generated images for one file can be overwritten by another with the same
name when mapping mime types for generated images
----------------------------------------------------+---------------------
 Reporter:  ianmjones                               |       Owner:  (none)
     Type:  defect (bug)                            |      Status:  new
 Priority:  normal                                  |   Milestone:  5.8.1
Component:  Media                                   |     Version:  5.8
 Severity:  normal                                  |  Resolution:
 Keywords:  has-patch needs-testing has-unit-tests  |     Focuses:
----------------------------------------------------+---------------------

Comment (by ianmjones):

 Replying to [comment:25 azaozz]:
 > 53668.4.diff looks better but I still think that it's not good to run
 `wp_unique_filename()` recursively:
 > - It's slower.

 I suspect it will be a tiny bit slower if the filter is implemented and
 there's work to be done, but I doubt it's that much.

 > - It would fire the filters several times for the same file which might
 cause problems.

 Each filename will be different, and this already happens, never seen a
 problem. What kind of problem do you envision?

 >
 > Also, this won't work well imho:
 > {{{
 > // If filename already versioned, get version and un-versioned filename.
 > if ( preg_match( '/-(\d)$/', $fname, $matches ) ) {
 >      $fname  = preg_replace( '/' . $matches[0] . '$/', '', $fname );
 >      $number = (int) $matches[1];
 > }
 > }}}
 >
 > It may match an uploaded file's name and we'll end up changing the
 original file name instead of appending a dash and a number. This can get
 pretty "messy" when somebody tries to upload files named something like:
 `text-1.pdf`, `text-2.pdf`, `text-3.pdf`, etc.
 >
 > Ideally the increasing of $number and the renaming should happen all in
 the same loop. That would ensure simplicity and consistency. When checking
 alternate file names all of them, including the original name, have to be
 checked at the same time. Cannot be done in a loop one by one as there may
 be "gaps" in the numbering of existing files.
 >
 > For example: existing files like img.jpg, img-1.png, img-2.webp,
 img-3.png and converting PNG and JPEG to WebP, and uploading img.jpg
 again. Then the sub-sizes of img-3.png will be overwritten if the alt name
 with .png extension is checked before the alt name with .webp (I'll try to
 make another test for this edge case).

 As mentioned in your PR, I don't think my patch has the problem you
 mention here with regards to skipped png version, recursion saves the day!
 😉️

 I see your point about the code you mentioned though. If uploading a new
 series of photos (pic-1.png, pic-2.png, pic-3.png) and the second
 "pic-2.png" clashes with "pic-2.jpg", what do you do?

 a) pic-1.png, pic-2-1.png, pic-3.png
 b) pic-1.png, pic-3.png, pic-4.png

 I take it (a) is expected?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/53668#comment:26>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform