Improve Dynamic Page Cache headers for HTML and JSON 4xx responses [#3451483] | Drupal.org

Improve Dynamic Page Cache headers for HTML and JSON 4xx responses [#3451483] | Drupal.org
Skip to search
Can we use first and third party cookies and web beacons to
understand our audience, and to tailor promotions you see
Improve Dynamic Page Cache headers for HTML and JSON 4xx responses
Fixed
Project:
Drupal core
Version:
11.x-dev
Component:
dynamic_page_cache.module
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
mxr576
Created:
31 May 2024 at 07:12 UTC
Updated:
16 Apr 2026 at 12:02 UTC
Jump to comment:
Most recent
Most recent file
Problem/Motivation
This was originally opened in the context of
#3278759: Access cacheability is not correct when "view own unpublished content" is in use
(see in particular
) and came up again in
#3580545: Make empty route lookup cacheable
Currently 40x responses have a very inconsistent behavior in terms of the
X-Drupal-Dynamic-Cache
Dynamic Page Cache response header:
Sometimes there is no header at all
Sometimes it is HIT which is somewhat confusing because the reponse itself is not cached (but there is a sub-request which may be cached and its cache header leaks into the main response)
Sometimes it is UNCACHEABLE which seems correct (but again is just leaked from the sub-request so is somehwat coincidental)
Steps to reproduce
Request a 404 page in HTML and JSON format for anonymous and authenticated users and be throughly confused about the different values of the
X-Drupal-Dynamic-Cache
response header.
Proposed resolution
We already detect the 40x (and also 50x) responses in
DynamicPageCacheSubscriber
. Move this code to run first to prevent any leakage of the sub-request-response to the main response and - in the case there was a sub-request - handle this explicitly. Concretely:
Always return a
UNCACHEABLE (403)
response headers for 403s (and similar for responses with a different 40x/50x status code) as the value of the
X-Drupal-Dynamic-Cache
header
In case there was a sub-request (which is the case for GET requests with HTML responses), also return the cache status of that, for example:
UNCACHEABLE (403, sub-request: MISS)
As part of that various tests that explicitly test the value of the
X-Drupal-Dynamic-Cache
header are updated.
One incorrect test assertion from
StandardTestTrait
is removed. It was introduced all the way back in
#2429617-316: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!)
without much discussion and it is (and presumably always was) actually accidentally testing the leakage of the 403 sub-request-response header to the main response. It is not actually testing the profile page, as the comment claimed, but the access denied page, because the logged in user is not user 1 but
/user/1
is being accessed. Actually "fixing" that by fixing the user ID then yields the "UNCACHEABLE (poor cacheability)" response header, which makes sense because of the user cache context. So removing that whole assertion is the best course of action.
Remaining tasks
User interface changes
API changes
The
X-Drupal-Dynamic-Cache
response header value is changed for 4xx and 5xx responses. (See
above
for details.)
Data model changes
Release notes snippet
Comment
File
Size
Author
#20
3451483-nr-bot_u099e7h5.txt
666 bytes
needs-review-queue-bot
Issue fork
drupal-3451483
Show commands
Start within a Git clone of the project using the
version control instructions
Add & fetch this issue fork’s repository
Or,
if you do not have
SSH keys set up on git.drupalcode.org
Add & fetch this issue fork’s repository
1 hidden branch
3451483-html-json-handling-diff
plain diff
MR
!8240
Check out this branch for the first time
Check out existing branch, if you already have it locally
About issue forks
Comments
Comment
#1
31 May 2024 at 07:12
mxr576
created an issue. See
original summary
or
to post comments
Comment
#2
31 May 2024 at 07:20
mxr576
changed the visibility of the branch
3451483-html-json-handling-diff
to
hidden
or
to post comments
Comment
#3
31 May 2024 at 07:20
mxr576
changed the visibility of the branch
3451483-html-json-handling-diff
to
active
or
to post comments
Comment
#4
mxr576
Hungarian
Hungary
commented
31 May 2024 at 08:04
Title:
Dynamic Page cache handles HTML and JSON 403 responses differently
» Cacheable HTML and JSON responses handled differently when HTTP 4xx occurs
Issue summary:
View changes
Related issues:
#3278759: Access cacheability is not correct when "view own unpublished content" is in use
or
to post comments
Comment
#5
mxr576
Hungarian
Hungary
commented
31 May 2024 at 08:09
Priority:
Normal
» Major
@fabianx @wim.leers or others, what does "UNCACHEABLE" means exactly? Because the current fix in the blocked issue is changing the order of
if (!isset($this->requestPolicyResults[$request])) {}
and
if (!$this->shouldCacheResponse($response)) {n
. The other potential alternative is also adding
X-Drupal-Dynamic-Cache: UNCACHABLE
when
if (!isset($this->requestPolicyResults[$request])) {}
is true... but is not that rather "UNHANDLED"?
or
to post comments
Comment
#6
31 May 2024 at 08:11
mxr576
opened
merge request !8240
or
to post comments
Comment
#7
mxr576
Hungarian
Hungary
commented
31 May 2024 at 12:15
I see that I was not the only one who wanted to clarify how many "UNCACHEABLE" scenarios/reasons exists and explicit headers to early returns \
Drupal\dynamic_page_cache\EventSubscriber\DynamicPageCacheSubscriber::onResponse()
#2951814: Improve X-Drupal-Cache and X-Drupal-Dynamic-Cache headers, even for responses that are not cacheable
or
to post comments
Comment
#8
mxr576
Hungarian
Hungary
commented
3 June 2024 at 20:06
Issue summary:
View changes
or
to post comments
Comment
#9
mxr576
Hungarian
Hungary
commented
4 June 2024 at 08:26
Issue summary:
View changes
or
to post comments
Comment
#10
mxr576
Hungarian
Hungary
commented
4 June 2024 at 15:03
Issue summary:
View changes
Now that user view access is also going to vary per user, we also hit this bug there.
See
#3452426: Insufficient cacheability information bubbled up by UserAccessControlHandler
or
to post comments
Comment
#11
mxr576
Hungarian
Hungary
commented
4 June 2024 at 15:04
Related issues:
#3452426: Insufficient cacheability information bubbled up by UserAccessControlHandler
or
to post comments
Comment
#12
mxr576
Hungarian
Hungary
commented
5 June 2024 at 14:19
Issue summary:
View changes
or
to post comments
Comment
#13
quietone
commented
17 July 2024 at 10:39
Version:
11.0.x-dev
» 11.x-dev
or
to post comments
Comment
#14
17 July 2024 at 10:39
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
or
to post comments
Comment
#15
1 April 2026 at 04:52
tstoeckler
made their first commit to this issue’s fork.
or
to post comments
Comment
#16
tstoeckler
he/him
German
Essen, Germany
commented
1 April 2026 at 05:00
Title:
Cacheable HTML and JSON responses handled differently when HTTP 4xx occurs
» Improve Dynamic Page Cache headers for HTML and JSON
Issue summary:
View changes
Status:
Active
» Needs review
Coming over from
#3580545: Make empty route lookup cacheable
where I also stumbled upon this and fixed this, but wanted to split that part out in order to keep that issue a bit more on scope there. Adding my proposed fix to the issue summary, but that solution is just my proposal so far, so both for people here already and for those coming from
#3580545: Make empty route lookup cacheable
as well, feel free to suggest improvements/alternatives for the concrete response header string.
or
to post comments
Comment
#17
tstoeckler
he/him
German
Essen, Germany
commented
1 April 2026 at 05:01
Title:
Improve Dynamic Page Cache headers for HTML and JSON
» Improve Dynamic Page Cache headers for HTML and JSON 4xx responses
or
to post comments
Comment
#18
tstoeckler
he/him
German
Essen, Germany
commented
1 April 2026 at 05:18
Issue summary:
View changes
or
to post comments
Comment
#19
tstoeckler
he/him
German
Essen, Germany
commented
1 April 2026 at 05:50
Regarding the removed assertion in
StandardTestTrait
in case anyone is wondering: This was introduced all the way back in
#2429617-316: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!)
without much discussion and it is (and presumably always was) actually accidentally testing the leakage of the 403 sub-request-response header to the main response. It is not actually testing the profile page, as the comment claimed, but the access denied page, because the logged in user is not user 1 but /user/1 is being accessed. Actually "fixing" that by fixing the user ID then yields the "UNCACHEABLE (poor cacheability)" response header, which makes sense because of the user cache context. So as far as I can tell, just removing that whole assertion is the best course of action here.
or
to post comments
Comment
#20
needs-review-queue-bot
commented
11 April 2026 at 01:38
Status:
Needs review
» Needs work
Status
File
Size
new
3451483-nr-bot_u099e7h5.txt
666 bytes
The
Needs Review Queue Bot
tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the
Drupal Contributor Guide
to find step-by-step guides for working with issues.
or
to post comments
Comment
#21
tstoeckler
he/him
German
Essen, Germany
commented
11 April 2026 at 05:05
Status:
Needs work
» Needs review
Just a test bot hickup.
or
to post comments
Comment
#22
smustgrave
commented
13 April 2026 at 15:13
Should this get a CR since it's returning a different status code then before.
or
to post comments
Comment
#23
tstoeckler
he/him
German
Essen, Germany
commented
14 April 2026 at 08:42
Issue summary:
View changes
or
to post comments
Comment
#24
tstoeckler
he/him
German
Essen, Germany
commented
14 April 2026 at 08:54
So the status code doesn't actually change, just the value of the
X-Drupal-Dynamic-Cache
response header. Made the issue summary more explicit about that. Also added a draft change notice for the change just in case, feel free to disregard if it's not actually needed. (Also feel free to re-word/improve it in case it is warranted, I found it a bit hard to explain the actual change in an understandable way.)
or
to post comments
Comment
#25
berdir
German
Switzerland
commented
14 April 2026 at 08:59
Issue summary:
View changes
Status:
Needs review
» Needs work
This is just optional debug information and kind of self-explanatory. Not sure if that really needs a CR. That said, there are quite a lot of tests asserting this, maybe a few of them will need an update, so why not, setting to needs work for that, but then I think this is RTBC.
One last thing, could you add the comment on the user page test change either as a MR comment or the issue summary so it's easy to find for the core committer reviewing this?
or
to post comments
Comment
#26
tstoeckler
he/him
German
Essen, Germany
commented
14 April 2026 at 12:56
Issue summary:
View changes
Status:
Needs review
» Needs work
Fair enough, copied that comment to the issue summary.
Not sure what specifically you meant to set this to "Needs work" for. I did already write a draft change notice in case that's what you meant.
or
to post comments
Comment
#27
berdir
German
Switzerland
commented
14 April 2026 at 18:45
Status:
Needs work
» Reviewed & tested by the community
#25 was a crosspost with #24, that's why.
CR looks great to me.
I doubt this really qualifies as a major bug, the debug header is confusing, but it's just a debug header. But it's useful to have it fixed either way.
or
to post comments
Comment
#28
15 April 2026 at 07:34
catch
closed
merge request !8240
or
to post comments
Comment
#29
15 April 2026 at 07:36
catch
committed
b00e9a89
on
11.x
task: #3451483 Improve Dynamic Page Cache headers for HTML and JSON 4xx...
or
to post comments
Comment
#30
catch
he/him
commented
15 April 2026 at 07:36
Version:
main
» 11.x-dev
Status:
Reviewed & tested by the community
» Fixed
This looks good to me, didn't comment here but did comment on the changes in the other issue already. Committed/pushed to main and cherry-picked to 11.x, thanks!
or
to post comments
Comment
#31
15 April 2026 at 07:36
Now that this issue is closed,
review the
contribution record
As a contributor, attribute any organization that helped you, or if you volunteered your own time.
Maintainers, credit people who helped resolve this issue.
or
to post comments
Comment
#32
15 April 2026 at 07:36
catch
committed
496796e0
on
main
task: #3451483 Improve Dynamic Page Cache headers for HTML and JSON 4xx...
or
to post comments
Comment
#33
wim leers
Ghent 🇧🇪🇪🇺
commented
16 April 2026 at 06:31
Fantastic to see this improved! 🤩
The one thing that confused me in the change record was that it seemed to imply that the response header previously already included the status code as the reason for uncacheability. But the diff shows what really happened:
UNCACHEABLE (poor cacheability)
UNCACHEABLE (404)
IOW: the reason for the response being uncacheable was
wrong
, and it was kind of the catch all "poor cacheability", which is misinforming developers.
Makes sense!
(Did I understand that right? 🤞)
or
to post comments
Comment
#34
tstoeckler
he/him
German
Essen, Germany
commented
16 April 2026 at 07:05
Yes, that's 100% correct. Please feel free to improve the wording of the change record if you have any ideas. I definitely did not mean to imply what you seem to have gotten from it, but, yeah, kind of had a hard time putting such a technicality into words in a way that is understandable - and didn't quite succeed it seems, sorry about that.
or
to post comments
Comment
#35
wim leers
Ghent 🇧🇪🇪🇺
commented
16 April 2026 at 11:49
Please don't apologize! This is very hard to explain. If anything:
thank you
for even creating a CR! 😊 Otherwise I'd never have seen this 🤓
I updated the CR based on you confirming #33 is accurate, WDYT?
or
to post comments
Comment
#36
tstoeckler
he/him
German
Essen, Germany
commented
16 April 2026 at 12:02
Great, yes, that's more explicit, thanks!
or
to post comments
Contribution record
Change records for this issue
X-Drupal-Dynamic-Cache response header updated for 4xx and 5xx responses
Add child issue
clone issue
Related issues
Referenced by
#2951814: Improve X-Drupal-Cache and X-Drupal-Dynamic-Cache headers, even for responses that are not cacheable
#2973356: Cacheability information from route access checker access results are ignored by dynamic_page_cache
Infrastructure management for Drupal.org provided by
Need a Drupal 7 extended support partner? Consider Tag1.
News items
News
Planet Drupal
Social media
Sign up for Drupal news
Security advisories
Jobs
Our community
Community
Services
Training
Hosting
Contributor guide
Groups & meetups
DrupalCon
Code of conduct
Documentation
Documentation
Drupal Guide
Drupal User Guide
Developer docs
API.Drupal.org
Drupal code base
Download & Extend
Drupal core
Modules
Themes
Distributions
Governance of community
About
Web accessibility
Drupal Association
About Drupal.org
Drupal is a
registered trademark
of
Dries Buytaert