Problem/Motivation

Follow-up to #3381293: Aggregation not working css and js cache directories not being created. When a web server (e.g. nginx) is not configured to pass requests for aggregated asset files to Drupal's PHP controllers, CSS/JS aggregation silently fails. The aggregated files are never generated because the asset delivery controllers (CssAssetController, JsAssetController) are never invoked, and the page renders without optimized assets. There is currently no feedback to the site administrator that their server configuration is incompatible with asset aggregation.

Steps to reproduce

  1. Use a web server configuration that serves the assets:// directory (typically sites/default/files/assets/) directly from the filesystem without falling back to Drupal's front controller for missing files (e.g. an outdated nginx template missing the try_files directive for the assets path).
  2. Enable CSS and/or JS aggregation at /admin/config/development/performance.
  3. Observe that pages render without aggregated CSS/JS — the assets are never generated.

Proposed resolution

Add client-side validation on the Performance settings page that probes the server configuration when aggregation checkboxes are enabled. The validation issues a lightweight HEAD request to a URL under the asset controller routes:

  • The existing AssetControllerBase::deliver() is extended to detect HEAD requests and return an immediate 200 OK — confirming that the web server correctly forwarded the request to PHP.
  • If the web server is misconfigured (serving the assets directory directly from the filesystem), the non-existent file produces a 404, which the client-side script detects.

This approach reuses the existing asset routes without introducing new endpoints. The HEAD method is ideal because:

  • It tests the exact same try_files / rewrite path that real asset GET requests use.
  • It avoids the overhead of theme initialization, library resolution, and hash computation.
  • It provides an unambiguous 200 OK success signal instead of relying on 400 Bad Request as a proxy for "reachable".

Remaining tasks

TBD;

User interface changes

When a user checks the "Aggregate CSS files" or "Aggregate JavaScript files" checkbox on the Performance page, a status message appears inline indicating whether the server configuration supports asset aggregation.

API changes

-

Data model changes

-

Release notes snippet

-

CommentFileSizeAuthor
#18 3384272-18.patch7.6 KBvoleger
#18 drupal_validate_server_for_assets_access.gif1.23 MBvoleger
#16 4673.diff4.75 KBd.stoyanov
#12 drupal-3384272-nginx-aggregation-validation-12.patch4.56 KByannickoo

Issue fork drupal-3384272

Command icon 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:

About issue forks

Comments

voleger created an issue. See original summary.

voleger opened merge request !4673

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Assigned: voleger » Unassigned
Status: Needs work » Needs review

The idea is to check the response status.
If the server is configured to return the file's content directly without passing a request through the PHP application on an empty file, the status code 404 will be returned.
In other cases, the controller will generate the aggregated file if files exist to aggregate or status code 400 if the aggregation file fails to be generated. In short, a response status code is different from 404.
I provided initial implementation. Feedback is welcome.

catch’s picture

catch
he/him
Primary language English
commented

Could we also do this for image styles?

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs change record

Image styles would be amazing as that's bit me before.

Moving to NW for a change record though for the new setting.

orkutmuratyilmaz’s picture

orkutmuratyilmaz
Primary language Turkish
Location İzmir
commented

brilliant feature! thanks for the development:)

zcht’s picture

zcht commented

The feature is really great, at this point a note that the patch is not compatible with Drupal 10.2. see details here in the issue: #3409219: system.performance.yml no longer considered after update

socialnicheguru’s picture

socialnicheguru commented

I received two errors:

admin/config/development/performance|Warning: Undefined variable $config in Drupal\system\Form\PerformanceForm->buildForm() (line 170 of d10.2.x/html/core/modules/system/src/Form/PerformanceForm.php

admin/reports/status|1||Error: Call to a member function get() on null in Drupal\system\Form\PerformanceForm->buildForm() (line 170 of d10.2.x/html/core/modules/system/src/Form/PerformanceForm.php)

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs work » Needs review

Thanks for the feedback. Adding new form elements to the config form was not the best idea.
So, I removed the validation button and performed validation on the appropriate checkbox checked change state.
Please review.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented

Regarding #5: I filled the follow-up issue #3418840: Allow to validate Apache/Nginx configuration for image style files access for image style validation as it requires a different approach and different change scope to implement the new validation feature.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented

test cases using different NGINX_VHOST_PRESET env var in Docker4Drupal development stack:
- drupal9 or drupal8 validation will show the warning regarding server requirement;
- drupal10 validation will show that server configured properly

yannickoo’s picture

yannickoo
Location Berlin
commented
StatusFileSize
new drupal-3384272-nginx-aggregation-validation-12.patch4.56 KB

In case you need to apply the patch for a 10.1.x Drupal you can find the patch attached 😇

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Believe next step would be to get test coverage added.

socialnicheguru’s picture

socialnicheguru commented

it would be nice to know which tests are failing

socialnicheguru’s picture

socialnicheguru commented

I get a js error in the console and this error in my logs:

https://mysite/css/_validate.css|https://mysite/admin/config/development/performance|1||Symfony\Component\HttpKernel\Exception\BadRequestHttpException: The theme must be passed as a query argument in Drupal\system\Controller\AssetControllerBase->deliver() (line 132 of d10.2.x/html/core/modules/system/src/Controller/AssetControllerBase.php).

d.stoyanov’s picture

d.stoyanov commented
StatusFileSize
new 4673.diff4.75 KB

Adding the current state of the MR so that I can use it from composer patches securely.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Issue summary: View changes

Updated IS

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
StatusFileSize
new drupal_validate_server_for_assets_access.gif1.23 MB
new 3384272-18.patch7.6 KB

Fixed base assets controller check issues. Maybe it's worth updating to use the existing system library files for testing.
Current behavior looks like this (GIF).
Attaching the patch from the latest MR version.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Needs work » Needs review
dcam’s picture

dcam commented
Status: Needs review » Needs work

This was tagged for needing tests. I'm setting the status back to Needs Work for those to be added.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

majid.ali made their first commit to this issue’s fork.

majid.ali’s picture

majid.ali commented
Status: Needs work » Needs review

I have added test as mentioned in #20. 1 PHPUnit Functional Javascript 2/3 test failing but its irrelevant to the changes done here. Changing status to needs review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs tests +Needs issue summary update

Sorry can the summary be updated, not super clear based on the description vs MR. Example I see a new system.library entry but nothing is calling it.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs change record, -Needs issue summary update
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs screenshots, +Needs change record

This one seems close but not super clear. CR mentions it could be validated but how? There are no screenshots or steps that mention that. Same for summary here, so tagging for such

But rest appears addressed and seems close!