[5.x]: Resized image transforms render from a stale cached source after replacing an asset on a non-local filesystem

[ryanwebhub]

What happened?

Context

My client has a Google Cloud Storage filesystem set up with her Craft 4 site (more on the v4 aspect later). She finds that replacing a file correctly updates the original, but some of the resized transforms still show the old image afterward. The transform files get a fresh timestamp, but the pixel content is stale. The problem still exists on Craft 5.10.10.

Description

Once Craft downloads and caches a local copy of an asset's original file for transform generation (for filesystems that aren't \craft\base\LocalFsInterface), it keeps using that cached copy indefinitely. \craft\helpers\ImageTransforms::getLocalImageSource() only re-downloads the source when the cached file is missing or 0 bytes, it never checks whether the cached copy still matches the remote object. If the remote object changes after the copy is cached, every transform generated from that point on is rendered from the outdated bytes, even though the transform file itself gets a fresh timestamp.

In practice we hit this after using the "Replace file" action on an asset stored on Google Cloud Storage. Craft does correctly clear the cached source on replace, via \craft\elements\Asset::_relocateFile() calling \craft\services\ImageTransforms::deleteAllTransformData(). But in the brief window between that clear and the new file becoming live, a concurrent transform reading (in our case, a duplicate ResaveOptimizedImages job from the ImageOptimize plugin, though it could be any transform request, a page render, a control panel thumbnail) downloads the still old remote object and re-seeds the cache with it. From then on, every variant is generated from that stale copy until something else happens to clear it.

Steps to reproduce

1. Configure an asset volume on a non-local filesystem (S3, GCS, or an S3-compatible store such as MinIO).
2. Upload an image and load it once, so Craft generates and caches a local copy at storage/runtime/assets/sources/<assetId>.<ext>. Viewing any transform, or the control panel thumbnail, is enough.
3. Without going through Craft, overwrite that same object in the remote bucket with a different image, using the same key so the asset's path is unchanged.
4. Request a new transform size for that asset that hasn't been generated yet, or force regeneration of an existing one by deleting its row in imagetransformindex and deleting the existing variant file.
5. Inspect the resulting transform.

Expected behavior

The transform reflects the current contents of the remote object, since Craft has to fetch the bytes when it caches a local copy in the first place.

Actual behavior

The transform is rendered from the old image. The transform file itself has a fresh timestamp, but its pixel content matches the object that used to be at that remote path, not the one that's there now.

Additional info

This only affects non local filesystems. Local disk volumes read the file directly through \craft\elements\Asset::getImageTransformSourcePath() and skip this caching path entirely. I think this might be related to issue nystudio107/craft-imageoptimize#391.

My client's site is still on Craft 4, but I learned along the way that the bug wouldn't be fixed by simply updating to the latest version of Craft 5 (and Craft 4 is at EOL, so no new patches). I used cweagans/composer-patches to offer her the code to patch the core, since no event listeners would do the job.

Here's the patch I gave my client that applies inside \craft\helpers\ImageTransforms::getLocalImageSource(), the first change roughly around line 150 and the second around line 191:

--- a/src/helpers/ImageTransforms.php
+++ b/src/helpers/ImageTransforms.php
@@ -150,7 +150,24 @@
         try {
             if (!$volume->getFs() instanceof LocalFsInterface) {
                 // This is a non-local fs
-                if (!is_file($imageSourcePath) || filesize($imageSourcePath) === 0) {
+                $fs = $volume->getFs();
+                $assetPath = $asset->getPath();
+
+                try {
+                    $remoteMtime = $fs->getDateModified($assetPath);
+                } catch (\Throwable) {
+                    $remoteMtime = null;
+                }
+
+                // Stale if the cache's stamped mtime no longer matches the remote's.
+                // Compares for inequality rather than "remote is newer than cache",
+                // both timestamps are only second resolution and a stale re-seed
+                // can otherwise land in the same second as a legitimate one.
+                $sourceIsStale = $remoteMtime !== null
+                    && is_file($imageSourcePath)
+                    && filemtime($imageSourcePath) !== $remoteMtime;
+
+                if (!is_file($imageSourcePath) || filesize($imageSourcePath) === 0 || $sourceIsStale) {
                     if (is_file($imageSourcePath)) {
                         // Delete since it's a 0-byter
                         FileHelper::unlink($imageSourcePath);
@@ -190,6 +207,11 @@
                     // we've downloaded the file, now store it
                     self::storeLocalSource($tempFilePath, $imageSourcePath);
 
+                    // Stamp the cache with the remote's own mtime, so the check
+                    // above compares like with like on the next read.
+                    if ($remoteMtime !== null) {
+                        @touch($imageSourcePath, $remoteMtime);
+                    }
+
                     // And delete it after the request, if nobody wants it.
                     if (Craft::$app->getConfig()->getGeneral()->maxCachedCloudImageSize == 0) {
                         FileHelper::deleteFileAfterRequest($imageSourcePath);

Craft CMS version

5.10.10. Also confirmed on v4.18.5, as that's my client's actual version. I realized having her upgrade to Craft 5 wouldn't solve the bug.

PHP version

8.4.23 in production. Also reproduced locally under PHP 8.4

Operating system and version

Linux, running in Docker containers. Production runs on Google Kubernetes Engine, our local reproduction used ddev on macOS.

Database type and version

MySQL 5.7.42 in production, also reproduced locally with MySQL 8.0. I can confirm the database is not involved in this issue.

Image driver and version

Imagick 3.8.1, ImageMagick 6.9

Installed plugins and versions

• nystudio107/craft-imageoptimize 4.0.14
• Blitz 4.23.19

These are the two relevant plugins, though I've confirmed the bug reproduces with only native Craft transforms and no plugins active at all.

[linear-code]

CMS-2306

[brandonkelly]

Thanks for reporting that! Fixed for the next release.

[brandonkelly]

Craft 5.10.13 is out with that fix. Thanks again!