Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Drupal 9.3 has an incompatibility with S3FS 8.x-3.0-beta3 due to a change to the "asset.css.optimizer" service described here https://www.drupal.org/node/2940031
TypeError: Argument 1 passed to Drupal\s3fs\Asset\S3fsCssOptimizer::__construct() must implement interface Drupal\Core\Config\ConfigFactoryInterface, instance of Drupal\Core\File\FileUrlGenerator given, called in /var/www/webroot/core/lib/Drupal/Component/DependencyInjection/Container.php on line 262 in Drupal\s3fs\Asset\S3fsCssOptimizer->__construct() (line 26 of /var/www/webroot/modules/contrib/s3fs/src/Asset/S3fsCssOptimizer.php).
It will crash with WSOD on page load and error on drush without remediation except to omit the S3FS configuration from settings.php.
Steps to reproduce
Upgrade to Core 9.3 and then try to use a configured previously working S3FS configuration.
Proposed resolution
Update S3fsCssOptimizer to use the new CssOptimizer's expected FileUrlGeneratorInterface instead of the ConfigFactoryInterface
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | incompatibility_with-3253414-11.patch | 8.93 KB | cmlara |
| #10 | interdiff-3253414-7-8.txt | 1.36 KB | cmlara |
| #8 | incompatibility_with-3253414-8.patch | 2.49 KB | cmlara |
| #7 | 3253414-7.patch | 1.13 KB | darvanen |
Issue fork s3fs-3253414
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3253414-incompatibility-with-drupal
changes, plain diff MR !13
Comments
Comment #3
stevenlafl commentedHere is a patch for 8.x-3.0-beta3
Comment #4
cmlaraI’m going to have to think on this one. (Sending from phone, not looked at code fully)
I don’t think we can just roll this patch into the existing release as the patch as is will likely break compatibility with older Drupal releases.
Need to sit down at console to look. We may just want to override the service deceleration for now and accept that we have a deprecated call or expand it to if the service exists optionally pass on the new file creation service.
And of course this is code that isn’t well exercised by tests so It did not show up when I ran https://www.drupal.org/pift-ci-job/2252746 against D9.3.0-beta1 (note to self start working on the 4.x tests )
Minor mitigation: I believe this should only impact sites with public:// takeover enabled
Comment #5
darvanenThis should be 8.9.x compatible, sorry I can't give an interdiff, I started from scratch.
I found that using the injected file_url_generator service at line 60 of S3fsCssOptimizer broke the test because it changed the output. Suggest that be a follow-up issue.
Comment #7
darvanenBugger.
Here's a less ambitious version that doesn't introduce the (very new) interface to S3fs.
I put the parent call in to trigger the deprecation message so we don't lose track of it, happy for that to disappear if need be.
Comment #8
cmlaraQuick mock addition to #7 to allow tests to pass with D9.3.
Discussed on slack, this is intended to be a 'fast patch' to prevent WSOD for D9.3 users, we will come back and do this right to use the new service.
We can't just inject the service as done in #5 due to issues seen in #3038524: s3fs, cdn, advagg integration
Thanks to @darvanen for working this out to get us to a temp fix for all Drupal versions s3fs currently lists as supported and @stevenlafl for the quick and detailed report.
Comment #10
cmlaraAttaching missing interdiff from #8.
Released 8.x-3.0-alpha4 with #8 included.
Downgrading to normal now that we have a stopgap measure in place.
I'm going to leave this issue open for now as a note that we still have more to work on.
Comment #11
cmlaraAttached is a more comprehensive version that allows us to utilize the new service (avoiding deprecation warnings) while retaining backwards compatibility with older versions of Drupal.
Honestly looking at this I believe that with regards to rewriteFileURI() we should do one of the following:
All of those options except full duplication may require a decent rewrite to S3fsCssOptimizer::. I'm not really fond of duplicating the code unnecessarily either making a trait seem like a good idea however that too may require some rewriting.
Additionally given https://www.drupal.org/about/core/policies/core-change-policies/drupal-8...
Along with
I think there are a number of areas in the code (CSS optimizer, Download Controller, etc) that we should consider avoiding inheriting and instead duplicate the code so that we have better control. This however is much more than I want to change about s3fs in the 3.x release at this time given where it is in the lifecycle and i suspect may be better done as part of the 4.x initiative.
I'm not really sure we should be replacing the class of the 'asset.css.optimizer' service instead of decorating the service (even if we never call the parent service) to reduce the risk of ever hitting a time where some other module considers doing the same or core makes a constructor change. Decorating priority would become a factor though making such a change more risky.
So in summation:
This could be better, but lets consider this patch for 3.x and focus on doing better in 4.x.
No interdiff due to a new commit base.
Happy holidays all, not expecting any comments from anyone this week, we are in a stable spot this can wait as a lower priority.
Comment #12
darvanen@dww would say "we haven't yet reached the rule of 3" and I'm inclined to agree, particularly because, as you say, a lot of work is going into 4.x right now.
I agree that 4.x is also the place to be considering architectural changes, not here.
Comment #14
cmlaraGood point on the rule of 3, jumping ahead on it is what makes this portion of the code such a mess when it it perhaps could of been simpler.
Committed the revised patch and closing for now. Will pickup the cleanup in the 4.x branch.