Problem/Motivation

Cache metadata on file entities, that have been uploaded through the text editor, are not bubbling up to the containing node.

I came across this issue whilst working on #2772847: Add support for private uploads / presigned URLs. Our S3 image urls have a signature that expires after a certain amount of time, so we are setting a max-age on the file entity. This however is not working for images uploaded through the texteditor, resulting in expired url signatures and 403s on images.

Steps to reproduce

1) Enable image uploads in one of your text editor settings.
2) Set a max-age on the file entity of 60 seconds (do some logic that changes every 60 minutes, such as dynamically changing url parameters).
3) View the rendered node with render caching enabled
4) Wait 60 seconds

Expected result:
The cache for the file entity + node should regenerate

Actual result:
The previous cache is used.

Proposed resolution

Set the file as a cacheable dependency of the render array.

Remaining tasks

  • Fix this in code.
  • Write tests

User interface changes

n/a

API changes

n/a

Data model changes

n/a

Release notes snippet

CommentFileSizeAuthor
#2 3228329-drupal-core-files-uploaded-through-text-editor-cache-dependency-2.patch648 bytesleon kessler

Comments

Leon Kessler created an issue. See original summary.

leon kessler’s picture

leon kessler commented
StatusFileSize
new 3228329-drupal-core-files-uploaded-through-text-editor-cache-dependency-2.patch648 bytes

Might be as simple a change as this.

I do wonder why it wasn't already set like this though?

I had a quick look at writing tests, there is already some basic tests for cache tags in EditorFileReferenceFilterTest, however I was unable to set a max-age through this test. So may have to use a test module to do this (through hook_file_load()).

cilefen’s picture

cilefen commented

I do wonder why it wasn't already set like this though?

git blame

leon kessler’s picture

leon kessler commented

I did look at git blame. The code is pretty old, from 2014.

https://git.drupalcode.org/project/drupal/-/blame/9.2.x/core/modules/edi...

I do wonder if the cache system was much more limited back then.

cilefen’s picture

cilefen commented

It actually looks like you have to look further back in time because that commit was a response to a method rename. But, yes, this is reaching pre Drupal 8 launch era.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Files uploaded through text editor do not have their cache dependency set properly » EditorFileReference filter only adds cache tags for referenced files, not contexts + max-age
Priority: Normal » Major
Issue tags: +D8 cacheability, +Contributed project blocker, +scalability

This bug is still relevant.

The thing is though that File entities in Drupal core have no cache contexts or max-age specified, which is why this went unnoticed.

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.