Contrib.social

Image Styles Entity wrongly assumes styles folder always contains image styles.

Open on Drupal.org →
Created on 7 June 2022, about 3 years ago
Updated 23 June 2025, 1 day ago

Problem/Motivation

The code at https://git.drupalcode.org/project/drupal/-/blob/fd92a54070af95150b3e227... assumes that all read/write filewrappers will have image styles stored stored at the uri wrapper://styles/{$imagestyle->id()}. This is not always the case, particularly when modules create custom filewrappers such as flysystem → . While this works most of the time, this could be a wrong assumption when the site needs to mount something like a FTP filesystem that just so happens to have a folder named "styles" and a subfolder with the image style id that contains things like CSS files or the like. This could result in irreparable file deletion. This happened to happen for us when we had a filewrapper that pointed at a shared s3 bucket.

While this can only be achieved using Contributed modules currently, I think it best we think about this possibility as a core issue as I find the logic overreaching.

A practical application for when this happened was when we made a clone of the production streamwrapper to allow flysystem to allow us to copy files from production to local enviornments. It then turned out that anytime we altered a image style setting to a image that was pointing on our "local" filesystem, we were inadvertently destroying production image derivatives with the flush that occurred on our local environment, even though all the files in files_managed had the stream wrapper for the local file environment.

Steps to reproduce

  1. Run a default standard drupal site installl
  2. have at least 1 image style (large, medium, thumbnail, and wide are defaults insalled with standard)
  3. Upload 1 image to the "image" field at /node/add/article using the default article content type
  4. Install Flysystem
  5. create a schema inside your settings.php to point to separate local folder such as the following example:
      $settings['flysystem'] = [
    'local-example' => [
      'driver' => 'local', 
      'config' => [
        'root' => 'sites/default/files/flysystem',
        'public' => TRUE,
      ],
  ];
  6. Clear cache
  7. Create a folder sites/default/files/flysystem/styles/large
  8. touch the file sites/default/files/flysystem/styles/large/styles.css
  9. navigate to /admin/config/media/image-styles/manage/large/flush
  10. click "flush"
  11. Validate that sites/default/files/flysystem/styles/large/styles.css no longer exists.

I feel it should be the expected results that styles.css still exists, furthermore, since the image was uploaded to public:// that the flysystem:// filewrapper not be touched at all.

Proposed resolution

I propose to harden Drupal against future challenges like this when it inevitably allows multiple public file wrappers, to make a check against the database to see if any images are using the wrapper in the files_managed table. In that situation, lets then assume that it's ok to flush the image styles derivatives in that folder as a stopgap.

Potentially a feature to be able to select where images styles may be located could be a solution.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Active

Version

11.0 🔥

Component

image system

Created by

🇺🇸United States generalredneck Texas, USA 🇺🇸

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 1 day ago →
    🇦🇺Australia acbramley

    This popped up in bug smash today. I'm not really sure how we'd go about fixing this, but it does seem like a fairly big edge case.

    I'm not sure the db query idea would be viable for performance, it looks like flushing image styles in all stream wrappers has been a thing since D7 and probably earlier.

  • Comment 1 day ago →
    🇺🇸United States generalredneck Texas, USA 🇺🇸

    Yes it's an edge case, but with real world application on an education site i maintained.

    My proposal was a single query per wrapper. I don't know how that looks in the code after 3 years, but I feel that the performance implications haven't been fleshed out. Also since it's been a problem since drupals in inception doesn't make it correct... But possibly an acceptable bug/problem.

    At the very least we document and/report on the problem. Thoughts?

  • Comment 1 day ago →
    🇦🇺Australia acbramley

    @generalredneck I agree, sorry if it sounded like I was discarding the idea that this could be a bug. We should definitely see if we can at least warn users. I just don't know where/how that could be done

  • Comment 1 day ago →
    🇺🇸United States cmlara

    to make a check against the database to see if any images are using the wrapper in the files_managed table.

    Image styles may be generated for un-managed files as well (albeit not common), I don't see the files_managed table as a viable solution.

    We should definitely see if we can at least warn users. I just don't know where/how that could obe done

    We have had this subject come up for other issues in the past. Documentation has to start somewhere, this issue is as good as any other to do so, stick it somewhere in the README, or some "deployment guide" for Site Owners to note that '/styles/' is a reserved path on all streamWrappers.

    Potentially a feature to be able to select where images styles may be located could be a solution.

    ✨ Split ImageStyle into the config entity and a separate event-based image processing service Needs work would be the best for this, it would allow solutions for numerous concerns. The new FlushEvent it proposes likely could allow this to be handled per streamWrapper by later changing the streamWrapper spec to require each streamWrapper provider listen and perform the flush as needed.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024