[wp-trac] [WordPress Trac] #42437: Thumbnails can overwrite other uploads if filename matches

[WordPress Trac]

#42437: Thumbnails can overwrite other uploads if filename matches
--------------------------+-----------------------------
 Reporter:  Viper007Bond  |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Future Release
Component:  Upload        |     Version:  4.8.3
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |     Focuses:
--------------------------+-----------------------------

Comment (by azaozz):

 Replying to [comment:21 joemcgill]:
 > We could append a -1 only to the intermediate size that has the
 conflict, but then

 Yeah, don't think this is a good idea. All sub-sizes should be named
 consistently.

 > If we wanted consistent names, we would first need to loop through all
 names that are going to be created and check if any of those files already
 exist on the server before we generate the intermediate image files.
 >
 > > How about my other idea: if the original file name ends with something
 like -600x800.jpg rename it and append -1, -2, etc. the same way we do for
 all other naming conflicts in WP.
 >
 > My main concern with this approach is that there are too many
 unpredictable reasons people might have for intentionally naming files
 which match this pattern, and renaming files, as opposed to appending a
 -1, -2, etc. seems more destructive.

 Sorry but I'm not sure I understand what you wanted to say here:
 - "intentionally naming files which match this pattern". Assuming you mean
 the `filemane-600x800.jpg` pattern.
 - "and renaming files, as opposed to appending a -1, -2, etc. seems more
 destructive". What seems more destructive? Appending -1, -2, etc. is
 renaming?

 > I recommend a hash (or similar) approach for intermediate image file
 names because it would fulfill the following requirements:
 > 1. Modify intermediate image file names rather than original file names
 to avoid collisions, since those files are generated by WP.

 Few things:
 - We already modify the original file name when it collides with
 previously uploaded file. Try uploading the same image few times and watch
 your `uploads` dir.
 - The idea to append -1, -2, etc. to the file name extends that original
 functionality. Introducing yet another "naming convention" to handle
 pretty similar circumstances seems like not so good.
 - Adding a hash to the sub-size names makes pretty ugly URLs. I'm not sure
 what the final impact may be but readability of URLs is a major
 consideration for all sites, not only for SEO.

 > 2. Ensure originality without the need to calculate and check for
 collisions during the process of generating intermediate images (a process
 that is already long-running and resource intensive on shared hosting,
 which can cause failures. See #40439).

 Right. No matter how we modify these names there will always be a tiny
 chance of name collision. We may minimize it by adding hashes, but it can
 never be 0, there will always be something like 0.00001%. In WP terms
 (tens of millions of installs) this is not such a small number :)

 > 3. Keep names of all intermediate image sizes in a set consistent so we
 can use pattern matching to detect the difference between an image that is
 part of an autogenerated set and ones that have been manually cropped in
 the WP media editor (important when calculating sources for `srcset`).

 Using pattern matching is actually easier done when the original filename
 matches most of the sub-size filename, like it is at the moment. Adding a
 hash makes that harder/slower.

 > It's also worth noting that WordPress already uses hashes in filenames
 if someone has the `IMAGE_EDIT_OVERWRITE` constant set to `false`. See
 [https://github.com/WordPress/WordPress/blob/4.9.5/wp-admin/includes
 /image-edit.php#L807-L837 wp_save_image()].

 That's not exactly the case. When an image is edited, a new image is
 created that has a hash in the filename, then all the sub-sizes are
 created from that new image. If we add another hash for sub-sizes, that
 will mean edited images sub-sizes will have double hashes, i.e. their
 filenames will be something like
 `20170729_142638-02332t348y5767374-e1526551747486-206x300.jpg` :)

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