Fix #1236: keep the companion files WP 7.1 records on an attachment - #1241
Conversation
WordPress 7.1 can keep the file an upload started from next to the one it serves, a HEIC beside its JPEG or a GIF beside its video, and records the name in the attachment metadata. Regenerating thumbnails replaced that metadata wholesale, and wp_generate_attachment_metadata() never produces those keys. Restoring a media therefore dropped them, and wp_delete_attachment_files() could no longer find the files: they stayed on disk for good, even once the attachment was deleted. Carry them over, and guard the metadata write against a non array.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
🟢 Coverage 73.33% diff coverage
Metric Results Coverage variation Report missing for cd27ae41 Diff coverage ✅ 73.33% diff coverage (50.00%) Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (cd27ae4) Report Missing Report Missing Report Missing Head commit (65851c7) 20327 1551 7.63% Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#1241) 15 11 73.33% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Description
Fixes #1236
WordPress 7.1 can keep the file an upload started from next to the one it serves: the HEIC a photo was uploaded as, beside the JPEG the site delivers, or the original GIF beside the video it was turned into. The file name lives in the attachment metadata and nowhere else.
Regenerating thumbnails replaced that metadata wholesale, and
wp_generate_attachment_metadata()never produces those keys, so restoring a media dropped them.wp_delete_attachment_files()reads them to clean up, so the files became invisible to WordPress and stayed on disk for good, even once the attachment was deleted.Nothing is visible to the user either way, which is exactly why it needed fixing: the only symptom is disk usage that grows and is never reclaimed on sites where people upload from phones.
Type of change
Detailed scenario
What was tested
Everything below was done by hand on a real WordPress 7.1-RC3-63235 build, in Chrome 151, with Imagify connected and backup on. A small mu-plugin was used during testing to read the attachment metadata back and to invoke the restore. It is not part of this PR.
Reproduced the bug first, from a real HEIC upload. On an unpatched build, uploaded a HEIC file through the block editor. The browser converted it to JPEG and WordPress kept the HEIC beside it, recording
source_image: test-source.heicin the metadata. Clicked "Restore Original" in the Media Library. The key was gone, and the 184KB HEIC was still sitting in the uploads folder with nothing referencing it. That is the bug: the file can no longer be found, so it will never be cleaned up.Verified the fix, again from a real HEIC upload. On this branch, uploaded a HEIC through the block editor. The browser converted it and sideloaded the source file, and WordPress wrote
source_image: iphone-shot-1.heicthrough its ownfinalizeendpoint. Then ran a restore on that media:source_imageafter restoreiphone-shot-1.heicThe restore genuinely ran in both cases, confirmed by the thumbnails being rebuilt from zero to four sizes and the optimization data being cleared.
One honest limitation, worth knowing for QA. On this machine the browser side pipeline never reaches its final step for HEIC files. All eight sideload requests return 200 and the converted JPEG and its sub sizes land on disk, but the
finalizerequest is never sent, so the metadata is left empty. It happens the same way whether the file is dropped or picked through the file input, and it does not happen for JPEG or PNG uploads. Nothing in this PR affects it, and it happens on an unpatched build too.To get past it, the
finalizerequest the browser should have sent was issued by hand against the real REST endpoint, with the source file the browser had genuinely sideloaded. WordPress then wrotesource_imagethrough its own code. The endpoint rejects a file name that was not really sideloaded for that attachment, so this could not have been faked. Everything after that point, the restore and the metadata write, is the untouched plugin code path.Also checked the three keys together. WordPress 7.1 records
source_image,animated_videoandanimated_video_posterthe same way, and none of them are produced bywp_generate_attachment_metadata(). Seeded all three on an optimized media exactly asfinalize_item()stores them, ran a restore from the Media Library link, and confirmed all three survived while the sizes were correctly rebuilt.Automated checks on top of the manual work: 5 new unit tests (
composer test-unit -- --group MediaWP), full suite green at 495,composer run-stanclean, PHPCS clean on both changed files.How to test
Setup:
localhost. Client side processing needs a secure context, and it is enabled by default there in any browser.sips -s format heic photo.jpg --out photo.heic.Upload it:
/imageand pressing Enter.Confirm the starting state:
wp post meta get <ID> _wp_attachment_metadata --format=json.source_imagekey naming the HEIC file. If it is missing, the browser did not finish the upload, so do not continue: the case under test is not set up. See the limitation noted above.wp-content/uploads/<year>/<month>/.Restore, which is what used to lose the key:
source_imagemust still be there, and must still name the HEIC file that is on disk. Losing it is the bug.Confirm the consequence is fixed:
Check nothing regressed on ordinary media:
Affected Features & Quality Assurance Scope
Media\WP::generate_thumbnails(), so re-optimizing with a different level and the missing thumbnails action.Technical description
Documentation
Media\WP::generate_thumbnails()writes the return value ofwp_generate_attachment_metadata()straight into_wp_attachment_metadata. That is correct for everything that function produces, but WordPress 7.1 added three keys it does not produce, written only when the browser handled the upload:source_image, the file the upload started from, kept when the browser converted it.animated_video, the video an animated GIF was turned into.animated_video_poster, the still frame for that video.All three name real files on disk, and all three are read back by
wp_delete_attachment_files()(wp-includes/post.php:6980and:7003) as the only record of what to clean up. Confirmed against core:wp-admin/includes/image.phpdoes not mention any of them.keep_companion_files()now carries them from the stored metadata onto the freshly generated array, before it is written. A key the new metadata already carries is left alone, so nothing is clobbered if a future WordPress starts producing them itself.While in there, the metadata write is guarded with
is_array(). WordPress 7.1 madewp_get_attachment_metadata()returnfalsefor non array metadata and coerce a non arraysizesto an empty array (wp-includes/post.php:7051and:7113-7141), so storing a non array value now reads back asfalserather than as what was written. Nothing in the plugin was known to write one, but the write was unguarded.The helper is
protectedrather than private so it can be exercised directly in the unit tests.New dependencies
None.
Risks
The change only ever adds keys that were already stored, and only when the fresh metadata does not have them, so it cannot alter sizes, file names or dimensions. On any WordPress below 7.1 the keys never exist and the helper is a no op.
Metadata already lost to an earlier restore is not recovered. The names are gone, so the files cannot be identified any more. This stops the loss from happening again rather than repairing it, and any such file has to be found on disk by hand. A migration would have to guess which stray file belonged to which attachment, which is not something worth risking.
Mandatory Checklist
Code validation
Code style
Unticked items justification
Nothing unticked.
Additional Checks
There is nothing to log: the outcome is observable directly in
_wp_attachment_metadata, where the keys either survive a restore or do not. No filesystem or HTTP calls were added, and the helper only reads two arrays.