Website error Exception: "Only file CSS assets can be optimized"

Created on 11 January 2024, over 1 year ago

Problem/Motivation

Similar to the cloned parent #3191028: Website error Exception: Only file CSS assets can be optimized but with a twist...

Since upgrading to 10.2 we've been seeing these in the logs (maybe because we moved away from advagg to core's aggregation)

We are seeing cached pages calling aggregated assets from the past! That triggers errors in the log as such:

Exception: Only file CSS assets can be optimized. in Drupal\Core\Asset\CssOptimizer->optimize() (line 43 of core/lib/Drupal/Core/Asset/CssOptimizer.php).

The problem was that we *HAD* some assets that were type: external AND minified: true

I fixed the problem by removing minified: true. But here's the twist: because these cached pages return cached aggregate assets that have includes the library that still exists, they assume they should still be minified because they are in the URL.

Steps to reproduce

I took the failing asset path locally (don't need the file to be there for test, it's the include= that causes the issue)
sites/default/files/css/css_LYCBGljzz7g174tLEdAJcO7-JDTovJeWZpQ-gEqd6Nk.css?delta=1&include=eJx1T4sOgzAI_KHWfpLByrBKHyvtnPv61cy4R7KEBO7gOLAxo5mvFfOmBrBLX2KLZD7qfpa_LWdjULJJQd94QUXAYEsV41EECEXZ3SHE7IHd42OAOA7AyvLlKLWUjV0ghffS8mLGXBNwd0BFMRJjX4AMFf8NgV4-hyTlSLkd8DvUwQz39wl2DLr56_bgm2t45y-rHlaVIANlSJOcq0-mqyHVgZ1MOH7r18mVgJse8fYEEqeHrA&language=en&theme=clf

Set a breakpoint here
public/core/lib/Drupal/Core/Asset/CssOptimizer.php:43

And this asset which exists still, but is not meant to be minified anymore is causing the exception to be thrown.

$css_asset = {array[8]} 
 type = "external"
 weight = {float} -199.999
 group = {int} 100
 data = "https://cdn.ubc.ca/clf/7.0.4/css/ubc-clf-full-bw.min.css"
 version = "7.0.4"
 media = "all"
 preprocess = true
 license = {array[3]} 

This is the library as it is now in galactus.libraries.yml:

clf-cdn-fw-bw:
  version: 7.0.4
  css:
    base:
      https://cdn.ubc.ca/clf/7.0.4/css/ubc-clf-full-bw.min.css: { type: external }
  dependencies:
    - galactus/cdn-clf-js

Proposed resolution

Consider silently aborting the library include if it's external and not minified (anymore)

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

🐛 Bug report
Status

Active

Version

10.2

Component
Asset library 

Last updated 2 days ago

No maintainer
Created by

🇨🇦Canada joelpittet Vancouver

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

Merge Requests

Comments & Activities

  • Issue created by @joelpittet
  • Status changed to Needs review over 1 year ago
  • 🇨🇦Canada joelpittet Vancouver

    Here's a patch to workaround this issue.

  • 🇨🇦Canada joelpittet Vancouver

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

  • Status changed to Needs work over 1 year ago
  • The Needs Review Queue Bot tested this issue.

    While you are making the above changes, we recommend that you convert this patch to a merge request . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

  • 🇨🇦Canada joelpittet Vancouver

    @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

  • Status changed to Needs review over 1 year ago
  • Pipeline finished with Failed
    over 1 year ago
    Total: 651s
    #80077
  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    Thanks for reporting. Will need a test case showing the problem.

  • 🇨🇦Canada joelpittet Vancouver

    @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
  • 🇺🇸United States smustgrave

    Maybe in Drupal\FunctionalTests\Asset;

    Also doesn't have to be it's own test, could be an additional assertion somewhere.

  • 🇺🇸United States firewaller

    Does it make sense to relocate this change to `\Drupal\Core\Asset\CssCollectionOptimizerLazy::optimizeGroup` similar to the JS approach? https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

  • I am reporting a similar (as it seems) issue when I visit the Resend page of a webform.

    It occurs with Claro and Gin themes. When I try with eg. the Bootstrap admin theme, the issue is not present.

    These are the errors I get:

    1. Only file JavaScript assets can be optimized. in Drupal\Core\Asset\JsOptimizer->optimize() (line 31 of /var/www/html/docroot/core/lib/Drupal/Core/Asset/JsOptimizer.php).

    2. Symfony\Component\HttpKernel\Exception\BadRequestHttpException: Invalid filename. in Drupal\system\Controller\AssetControllerBase->getGroup() (line 224 of /var/www/html/docroot/core/modules/system/src/Controller/AssetControllerBase.php).

  • 🇺🇸United States jlancaster

    Error logs on a few of my sites were getting filled with warnings and I could not find culprit in backtrace. Applying this patch to core (#23) has resulted in clean logs, finally!

  • Pipeline finished with Failed
    8 months ago
    Total: 570s
    #318817
  • Pipeline finished with Failed
    6 months ago
    Total: 681s
    #351295
  • First commit to issue fork.
  • Pipeline finished with Failed
    6 months ago
    Total: 471s
    #374742
  • 🇪🇸Spain ssantaeugenia

    Thanks for this great patch. I've attached the modified patch to also affect JavaScript.

  • Pipeline finished with Failed
    3 months ago
    Total: 110s
    #454360
  • 🇩🇪Germany osopolar 🇩🇪 GER 🌐

    I have the same errors in the log and now I am wondering how to fix it.

    @joelpittet You mentioned that you had assets that were type: external AND minified: true and so you removed the minimized there. We have one such asset, the better_exposed_filters library. But I am wondering why minified: true shouldn't be set for external libraries. There is a example of both attributes set in documentation under minified, with the description

    Whether the asset is already minified. Default: false

    The asset in your example (https://cdn.ubc.ca/clf/7.0.4/css/ubc-clf-full-bw.min.css) seems to be already minified, so why not setting minified: true?

  • 🇨🇦Canada joelpittet Vancouver

    @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.

  • 🇬🇧United Kingdom catch

    Yes minified describes the library, not what to do with it.

  • 🇺🇸United States firewaller

    At the very least the condition to check "external" should be considered in \Drupal\Core\Asset\CssCollectionOptimizerLazy::optimizeGroup calls

  • 🇨🇦Canada joelpittet Vancouver

    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-...

  • Pipeline finished with Failed
    4 days ago
    Total: 90s
    #513527
  • Pipeline finished with Failed
    4 days ago
    Total: 681s
    #513529
  • 🇨🇦Canada joelpittet Vancouver

    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.

  • 🇨🇦Canada joelpittet Vancouver
  • Pipeline finished with Success
    4 days ago
    Total: 4125s
    #513560
  • 🇨🇦Canada joelpittet Vancouver
  • 🇨🇦Canada joelpittet Vancouver
  • 🇨🇦Canada joelpittet Vancouver

    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.

  • Pipeline finished with Success
    4 days ago
    Total: 1632s
    #513699
  • 🇩🇪Germany Anybody Porta Westfalica

    Thanks @joelpittet great work and test is green!

    I left some final, minor comments, what do you think?
    We're having these log entries day by day...

  • 🇨🇦Canada joelpittet Vancouver

    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.

  • Pipeline finished with Canceled
    3 days ago
    Total: 297s
    #514407
  • 🇺🇸United States nicxvan

    nicxvan changed the visibility of the branch 11.x to hidden.

  • Pipeline finished with Success
    3 days ago
    Total: 580s
    #514413
  • 🇺🇸United States nicxvan

    I think this is ready now, I compared with the JS lazy optimizer and the logic looks the same.

    I don't think bugfixes like this need CRs.

    Test only fails as expected.

    I took a chance on updating the title too.

Production build 0.71.5 2024