Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
When doing a scan we had some issues and it seems due to some of the security headers becoming "invalid" due to them being duplicated.
X-UA-Compatible: IE=edge, IE=edge
X-Content-Type-Options: nosniff, nosniff
X-Frame-Options: SAMEORIGIN, SAMEORIGIN
Steps to reproduce
Install bigpipe sessionless and inspect headers on one of the responses to a "primed" cached page (i.e. second request after the BigPipe response)
Proposed resolution
Unsure, it appears to be related to "filtering" the response prior to placing into page cache, which causes headers to be added a second time.
Remaining tasks
-
User interface changes
-
API changes
-
Data model changes
-
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | 3268665-14-test-only.patch | 1.12 KB | wim leers |
Issue fork big_pipe_sessionless-3268665
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:
changes
Comments
Comment #2
wim leersInteresting find! 🤓
Any chance you could write a test to reproduce this? 🤓
Comment #3
wim leersIt should be pretty easy to add tests for this: just needs a few extra assertions in
\Drupal\Tests\big_pipe_sessionless\Functional\BigPipeSessionlessTest::testBigPipeNoSession()AFAICT!Comment #5
driskell commented@wim-leers Got the failing test in there and fixed it roughly, not sure if it the best approach but essentially I think the goal is to keep a copy of the headers before filtering, so that they can restored when the cached page is built up. Seems to have the desired effect.
I don't know if this might interact weirdly if there's a response filter that acts on say, attachments, and then remove its and adds a header, or does something else, where by the time sessionless wants to filter it again the information is lost and the header isn't restored. Probably needs some thought but at least there's the failing test here and some initial solution.
Comment #6
driskell commentedWorth noting this reproduces in the existing test exactly as described. Thus it was a 1 line to check the header duplication.
Comment #7
driskell commentedComment #8
driskell commentedHaving thought about it, in context of https://www.drupal.org/project/big_pipe_sessionless/issues/3267819, I think there is more work to be done to rationalise the handling of the "original response".
It seems both issues are caused by a confusing implementation of "original response" that doesn't work in this case as it duplicates headers, but also doesn't work in the sub request instance. Perhaps the right fix would fix both of the issues.
Comment #9
wim leers#8 That sounds very likely indeed!
Would you be willing to expand the test coverage you wrote here with the additional test case you found in #3267819: Pages generated via subrequest never get cached? I think that'd make for an excellent addition to
\Drupal\Tests\big_pipe_sessionless\Functional\BigPipeSessionlessRegressionTest().In any case: thank you so much for this thorough research! 🙏
Comment #10
wim leersRelated: #2861552: Duplicate HTTP headers should be removed. We should test if that fixes it for BigPipe Sessionless too.
Comment #11
wim leersComment #12
driskell commentedI haven't empirically tested this but my assessment of the core change in https://www.drupal.org/project/drupal/issues/2861552 is that it only deduplicates headers configured via #attached, which are mostly Link headers.
The headers duplicated here are copied directly from the original response object to the new one and then pass through EventSubscribers that originally added them which then add them again. So I don't think that ticket fixes it. I also think it makes sense for Drupal to validate and deduplicate its own "header instructions" - but technically at the request/response level duplicating headers is normal and acceptable so it wouldn't make sense to reactively fix it at that level so not sure the fix would translate well
Comment #13
wim leersRight — I should've read the patch in detail, sorry! The titles just sounded so similar! 😬
Any chance you could write a failing test for this issue? That'd make this far easier to commit — the current MR looks clean (👏), but is far more complicated than the issue title would imply/suggest: it's not just simple deduplication! 😅
Comment #14
wim leersD'OH! The MR does have test coverage! 🙈
Converted that into a patch.
It's #8 that this issue is marked for, not for tests. So removing the tag. 👍
Comment #15
gaëlgUnfortunately, it looks like my fix in #3267819: Pages generated via subrequest never get cached does not solve this issue. But it might help?
Comment #16
wim leersRebased the MR.
Comment #17
wim leersI cannot get the test-only patch in #14 to fail locally. Can you? 😅
Comment #18
wim leersI bet this was caused by the #3214208: FinishResponseSubscriber could create duplicate headers bug in Drupal core 😅.
That would explain why I could not reproduce this in #17.
Can you please try to reproduce this manually with that core patch applied?
Comment #19
wim leers