Add support for minified external CSS libraries

Here's a patch to workaround this issue.

Ideally it would be nicer to consider the include invalid cache and rebuild the asset or something...

@djsagar Why move it to 11.x-dev it's a bug, did the policy change or doesn't it get applied to the latest release's branch? I maybe missed a memo

Found the memo :) https://www.drupal.org/node/767608#d8-minor

@smustgrave any hints at where tests of this nature should go or could extend?

Test Scenario:

• External Library + minified included in the URL = Fail as expected
• External Library + without minified included in the URL = Skipped

@osopolar that’s a really good point — I should know better than to make assumptions about how this works. I was under the impression minified was an action to be taken rather than a descriptor. But you’re right, the docs clearly state otherwise. Looks like I’ve been shooting myself in the foot here! Thanks so much for pointing it out.

That said, I still think the page-cached asset URLs are a problem, regardless of my incorrect understanding. I’ll try to update the issue summary to reflect that.

I’m digging into this because the error still appears in my logs month to month.

Here’s the asset that triggers the failure: core/lib/Drupal/Core/Asset/CssOptimizer.php:43

Array
(
    [type] => external
    [minified] => 1
    [weight] => -199.99963333333
    [group] => 100
    [data] => https://cdn.ubc.ca/clf/7.0.4-minimal/css/minimal-clf-full-7.0.4-bw.css
    [version] => 7.0.4
    [media] => all
    [preprocess] => 1
    [license] => Array
        (
            [name] => GPL-2.0-or-later
            [url] => https://www.drupal.org/licensing/faq
            [gpl-compatible] => 1
        )

)

This is coming off a request to:
/sites/default/files/css/css__xlnlyYvQDTExRxMzowYQQbU2Gkpxbvzgs_51zHi7ms.css?delta=1&include=eJxlj-EOwiAMhF-IySORwsqGFkpo56ZP79QZjfy7-y6X9iK3rKmKjYcwsCg7KGHmJvbXmACEZYRmP-KkM2Y0DaVykXRFp-AJXUyk2OyXDy9u5CaK2XoQNIEb2rIfBUp3NBMQBF3EBXG4qfXMKtqg9smz_Q8zisCE0gUTsQcyu6RULnZsSwU6Hfb9xMHgDFtXDxSHuA5-7ZJ9Iz4A2b18gw&language=en&theme=galactus_cs_ext

Which attempts to load a library from my theme:
galactus_cs_ext/clf-fw-bw
That library is no longer used. So this request seems to be coming from a cached, malicious, or stale reference, and causes the

“Only file CSS assets can be optimized”

error.

For reference, here’s how you can decode the include hash via the command line:
php -r "print_r(@gzuncompress(base64_decode(str_replace(['-', '_'], ['+', '/'], 'INCLUDE_HASH_GOES_HERE'))));" Or you can use UrlHelper::uncompressQueryParameter() in code.

I’m not sure how to categorize this type of bug—it stems from a library that used to be used, still exists, but is no longer active in the theme. Yet the request for it persists and causes an exception rather than a graceful fallback or warning.

You might see why I confused as to the "minified" as it looked like a type: "external" is being "minified", and is thrown in a if ($css_asset['type'] != 'file') {

The library definition is:

clf-fw-bw:
  version: 7.0.4
  css:
    base:
      https://cdn.ubc.ca/clf/7.0.4-minimal/css/minimal-clf-full-7.0.4-bw.css: { type: external, minified: true }
    theme:
      css/clf/clf.social-icons.css: {}
  dependencies:
    - galactus_cs_ext/cdn-clf-js

Our docs suggest to me that this looks right:
https://www.drupal.org/docs/develop/theming-drupal/adding-assets-css-js-...

Took a stab because I noticed in debugging JsCollectionOptimizerLazy had an exclusion that CssCollectionOptimizerLazy was missing.

Fixing that prevents this bug, I will see if I can write a test to prove it's fixed.

https://git.drupalcode.org/project/drupal/-/commit/b957382#2fb629f0e45c5... This is where the JS part got committed for reference: #1014086: Stampedes and cold cache performance issues with css/js aggregation

Ok wrote a test, hope the local file I faked as external is appropriate in this case. Couldn't think of what to do for an "external" CSS file but it doesn't really need to be truely external, just something to give to file_get_contents().

The tests-only fail (as expected) by the way.

Thanks @Anybody for review, I put code suggestions that you can apply (might need push access to do so), but feel free to apply the suggestions or make your own in the comments you started.

@herved Your expectation is how it should work, this change was creating parity in how CSS works as that is how JS works (I lifted the code from what we had in JS already). Can you revert the diff on your machine to see if it truely is this commit? (I was confused about "minified" coming into this issue in the first place, I think I got a better grasp coming out.

You can grab the commit URL and add .diff to the end to get a patch and reverse it like this:
curl https://git.drupalcode.org/project/drupal/-/commit/a1b3926d59e341d771963eb8a3de9d881be7a107.diff | patch -p1 -R
And after testing it re-patch
curl https://git.drupalcode.org/project/drupal/-/commit/a1b3926d59e341d771963eb8a3de9d881be7a107.diff | patch -p1

@herved and @benabaird I see what you mean—sorry to cause so much trouble with this, and thanks for following up (and for opening a new issue to track it).

I ran into the same thing with fontawesome as you did, @benabaird. When I wrote the patch and test, I copied how JS assets were handled, and there wasn’t anything in the comments or tests at the time to highlight this gotcha for CSS files—especially for local minified files relying on URL rewriting during aggregation.

I wouldn’t want this commit reverted, as it does solve a different and legitimate problem. But I do agree that targeting external assets specifically would be a more precise fix, and wouldn’t break local asset aggregation that people are relying on.

Appreciate you both raising this and suggesting a focused fix. I will see you over on the follow-up issue! #3535330: Assets paths in CSS no longer rewritten when aggregation is enabled