[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:21 azaozz]:
 >
 > Yes the alt patch needs more work.

 Happy to test and review your PR when it's ready, I'm `ianmjones` on
 GitHub.

 >
 > Looking at `_wp_check_alternate_output_format_uniqueness()` again, seems
 it tries to account for "circular" use of the `image_editor_output_format`
 filter. For example:
 >
 > {{{
 > Array(
 >     'image/png' => 'image/gif',
 >     'image/gif' => 'image/jpeg',
 >     'image/jpeg' => 'image/png'
 > )
 > }}}
 >
 > In that case uploaded PNG image will be converted to a GIF. There's no
 need to test for additional conversion settings. This is a weakness/lack
 of docs in the current filter implementation that needs fixing, the file
 naming shouldn't account for such cases.

 It does cover that scenario, but only as a side-effect of the thumbnail
 regeneration protection scenario I was going for.

 >
 > Then `_wp_check_alternate_output_format_uniqueness()` runs
 `wp_unique_filename()` in a loop after the original filename was made
 unique. However seems that loop will run only once as there's `static
 $checking_alternates;` which prevents calling `wp_unique_filename()`
 second time. That's okay, a loop doesn't seem needed. There can only be
 one conversion per uploaded image as explained above. In addition running
 all of wp_unique_filename() again seems not that efficient. That's why I
 started on an alt patch, to see if things can be simplified and more
 efficient :)

 There are two loops in my new code, one checks the results of
 `image_editor_output_format` to find all the extensions that need to be
 analysed, and the second to check them via `wp_unique_filename`.

 The first loop ensures that...

 1. The alternate thumbnail output format for the just uploaded file is
 checked for clashes.
 2. Any **other** upload file format that might also generate the same
 alternate thumbnail output format are checked for clashes.
 3. Any **other** upload file format that might generate the same thumbnail
 output format as the uploaded file's native format are checked for
 clashes.

 The static is there so that those loops are not run again when
 `wp_unique_filename` is called from within the second loop. I did it this
 way because `wp_unique_filename` works well for testing a single
 extension, and I did not want to complicate that function. The recursion
 into it is only one level as a result, the new function is effectively
 skipped on that second entry into `wp_unique_filename`, without needing to
 alter `wp_unique_filename` bar that one function call.

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