Regression: Auto-rotated or scaled image upload can overwrite previously uploaded image

[ianmjones]

It's very easy to upload an image that uses the new auto rotation that overwrites the original file of a previously uploaded image.

Steps to reproduce...

1. Upload a "normal" image with name image-rotated.jpg

This creates files such as...

2019/10/image-rotated.jpg <- base file (full)
2019/10/image-rotated-150x150.jpg <-thumbnail

2. Upload an image originally sourced from an iPhone that is named image.jpg and that will be auto-rotated.

This creates files such as...

2019/10/image-rotated.jpg <- base file (full)
2019/10/image-rotated-150x150.jpg <-thumbnail
2019/10/image.jpg <- original_image

That second auto-rotated 2019/10/image-rotated.jpg file has overwritten the original file from step (1).

Both Media Library items now point to the same 2019/10/image-rotated.jpg file in their _wp_attached_file and _wp_attahcment_metadata postmeta records.

This means image (1) is effectively lost and thumbnail regeneration is going to use the image from step (2).

Repeat with another-image-scaled-2560.png that is already scaled to the 2560 threshold (2560x1440), followed by another-image.png that will be rescaled from 5120x2880.

The 2019/10/another-image-scaled-2560.png file from step (1) will be overwritten by the auto-scaled file from step (2).

[desrosj]

Thanks for these detailed reports, @ianmjones!

I don't think this is a regression on 5.3, rather a long standing bug (see #42437). But, 5.3 does introduce a few more scenarios where an image could potentially be overwritten.

I think this can be closed as a duplicate since there is much more history over on #42437. Marking for a second opinion, but if that ends up being the resolution, @ianmjones please just pop over there and add your thoughts with the information you included above!

[ianmjones]

Sorry, missed fixing up a bit of copy-paste...

Replying to ianmjones:

This creates files such as...

2019/10/image-rotated.jpg <- base file (full)
2019/10/image-rotated-150x150.jpg <-thumbnail
2019/10/image.jpg <- original_image

Should be...

2019/10/image-rotated.jpg <- base file (full)
2019/10/image-150x150.jpg <-thumbnail
2019/10/image.jpg <- original_image

@desrosj I understand why you say it's not a regression, but seeing as we're talking about the auto-generated base file names that should definitely be run through wp_unique_filename before being used in _wp_attached_file, I believe this is a regression unique to the new rotation and scaling methods.

I think if #48453 is addressed in such a way as to properly make sure that the base file name is unique as it normally has been previously, then hopefully this particular ticket can be closed.

[desrosj]

Moving this into the milestone for some visibility during RC.

Please leave the keywords. Even though we feel a bit differently about the action required to resolve this, they help other contributors understand the status of the ticket and what is needed to move it along (a second opinion, as there is a recommendation to close).

[azaozz]

Yes, it is a long-standing problem that ideally should be fixed for all cases in #42437. One of the early patches for "big image" was using wp_unique_filename() for -scaled and -rotated but it was removed later in favour of fixing this for all image file names.

We can bring use of wp_unique_filename() back for -scaled and -rotated, however this will further complicate things for #48453. Ideally all of these cases will be fixed in #42437 during wp_unique_filename() that runs on the initial saving of the uploaded file, something like https://core.trac.wordpress.org/attachment/ticket/42437/42437.diff.

As this is closely related to #48453 lets keep it open for now and look at solution for both cases at the same time.

[azaozz]

Tend to agree with @desrosj here. Lets look at this in 5.3.1 and use #42437 to fix it.