Asset cache-control headers are incorrect

Created on 16 February 2025, about 2 months ago

Problem/Motivation

Broken off from πŸ› AssetRoutes::routes() assumes stream wrapper implements ::getDirectoryPath() Active .

AssetControllerBase sets the cache-control header to private, no-store however this becomes public, no-store due to Response::__construct() having a $public argument that overrides the value in cache-control.

This also reveals a lack of test coverage for this being set properly on the eventual response sent to the client.

Additionally, there is some question as to whether this is the "correct" header to set, particularly when it comes to situations where subsequent requests for the generated asset do not come from disk, but a CDN. This would happen in cases where the backing storage for assets is not publicly available, for instance.

Broken out to its own issue as the original issue is more narrow to the backing storage/stream wrapper question, and there needs to be some discussion here regarding the correct approach.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

11.0 πŸ”₯

Component

asset library system

Created by

πŸ‡ΊπŸ‡ΈUnited States bradjones1 Digital Nomad Life

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

Comments & Activities

  • Issue created by @bradjones1
  • πŸ‡ΊπŸ‡ΈUnited States bradjones1 Digital Nomad Life

    Copying over comment from @catch prior to this being split off:

    @bradjones1 the main reason to prevent sending cacheable headers there is to avoid, for example, different CDN endpoints from serving the PHP version of the response vs. the file as it's served from the web server direct from disk. Even if the content can be guaranteed to be exactly the same, the headers would be different.

    There are two reasons for this apart from consistency:

    I've seen situations in the past (maybe Drupal 7 era) where you end up with different versions of supposedly the same thing in different CDN endpoints and it was not enjoyable. Even something like different max age/expires headers could be annoying.

    Also some hosts, the only example I know of is Pantheon but there may be others, bill differently for requests served from PHP vs static assets from disk, even when those requests are cached in the CDN. I don't understand why they make that distinction but it's how I interpret this documentation:

    https://docs.pantheon.io/guides/account-mgmt/traffic#pages-served

    I will elaborate on my thoughts on this in a separate comment, but I have poked @greg.1.anderson at Pantheon for some clarification on this front, as it does seem a little curious that different cache entries would be billed differently on a CDN based on the origin.

  • πŸ‡ΊπŸ‡ΈUnited States bradjones1 Digital Nomad Life

    Regarding treating the response "from Drupal" differently from subsequent requests for the same file served off disk:

    I've seen situations in the past (maybe Drupal 7 era) where you end up with different versions of supposedly the same thing in different CDN endpoints and it was not enjoyable. Even something like different max age/expires headers could be annoying.

    Certainly this could be an issue, but we also need to consider the increasingly-common cloud native use case. As it stands, there's no out-of-the-box option for here site owners to deploy Drupal in a runtime with no publicly-accessible locally-persisted storage and enjoy the benefits of cache-friendly headers.

    The default use case is certainly the typical model of "public files on disk," and that should be what Drupal expects out of the box. What we should do, then, is at least support sending cache-control headers that are CDN-friendly for caching the "PHP response" without needing to patch core.

    This is expert-mode stuff so I don't think it would be out of school to put this behind a container parameter? (I really don't like introducing new "Settings settings" in 2025/Drupal 11, but I feel as though I may be tilting at a windmill there.) That would at least provide a hook-point for customizing this and serving both use cases?

Production build 0.71.5 2024