FinishResponseSubscriber: Need warning/error when headers exceed 16k

mr !143 apply and solve my problem on D9.5
Many thanks!!!

mr! 1943 Worked for me too on 9.3.22. Thanks!

Moving to NW for the test cases before reviewing.

Hello
I'm facing off this on D9.5.3, do I need to patch ? if yes, which files ?

Thanks

@aiphes : you must look at the mr !1943 (see #57 for the link). From there, you can download the patch (in the upper right part of the screen see Code > download email patches)

@Dunebl, so this is the patch https://www.drupal.org/files/issues/2021-12-09/2844620x93.patch → , and in which folder do I need to apply it ?

@aiphes this is not the last/full patch. Download it from the explanations I provide in #66 then apply it from within your Drupal root.

Here's a link to the MR:

https://git.drupalcode.org/project/drupal/-/merge_requests/1943

And here's a direct link to the patch:

https://git.drupalcode.org/project/drupal/-/merge_requests/1943.diff

I believe, security-wise, it's best practice to download the diff and use it a local patch file than reference this link, because attackers could update this MR to introduce vulnerabilities, and then you'd just automatically pull them in.

You can unpatch with this command:

cd "${drupal_root}"
patch -p0 -R < "${path_to_patch}"

@DuneBL & @charginghawk
Thanks for your help, patch seem to fix my random 500 error on editing certain content types. Hope this is durable. I will do the same on my others D9 websites.
Will I need to repatch after core updating in future ?

@aiphes: yes, you will need to repatch after each core update. (you can use composer and add the patch in composer.json)

@DuneBL for sure, but how can I do that ?

A quick search leads me to this article: https://duvien.com/blog/how-apply-patch-file-composer-based-drupal-89

ok, but I get errors of syntax:

 "extra": {
        "drupal-scaffold": {
            "locations": {
                "web-root": "web/"
            },
		"file-mapping": {
             "[web-root]/robots.txt": false,
			 "[web-root]/sites/default/default.*": false
			}
        },
		"patches": {
			"drupal/core": {
				"Patch pour Premature end of script headers - 2844620": "https://git.drupalcode.org/project/drupal/-/merge_requests/1943.diff"
			}
		}
    },

 "./composer.json" does not contain valid JSON
  Parse error on line 290:
  ...   ]        }    },    "require-dev":
  --------------------^
  Expected: 'EOF'

Um, sorry to interrupt the party, but I'd like to note that usually the patch is not needed for live websites. Make sure the development mode is not active (i.e. the line "include $app_root . '/' . $site_path . '/settings.local.php';" is commented/not enabled in sites/default/settings.php). Development mode should not be enabled for live websites! If you run into this problem with development mode enabled, make sure "http.response.debug_cacheability_headers" is set to false in sites/development.services.yml. Make sure you clear the cache after any changes in these files. These things should usually avoid this error - no need to patch anything.

@rgpublic So you tell that we doesn't need to use settings.local.php on production websites ?
Because I do this every time, whithout issues except this one after migrating from D6 or D8 to D9.

@aiphes: Exactly. You can open and read that file if you don't believe me. You will find that contains only things that are needed for *development*. It will significantly slow down your live website. It disables caches, enables debugging headers and so on. It will also leave your site vulnerable to some attacks. You *can* run with this enabled. But it's not recommended.

@rgpublic ok, I will plan to use the settings.php and see after the future update ( that will drop the patch) , my 500 errors still goes out. Finger cross.

@DuneBL Trying to rollback and it doesn't work
/sited9$ patch -p0 -R < "https://git.drupalcode.org/project/drupal/-/merge_requests/1943.diff" or

/sited9/web$ patch -p0 -R < "https://git.drupalcode.org/project/drupal/-/merge_requests/1943.diff"

give
-bash: https://git.drupalcode.org/project/drupal/-/merge_requests/1943.diff: No such file or directory

@aiphes I don't think that the patch command works with https. You should use the real patch file in your local hard drive

@DuneBL But I don't know where are stored patches. I've nothing in the ~/patchs folder :/ I go to ask to my web host.

There is a misunderstanding: you must download by yourself the patch file and put it in the HD of your web host. After that you can run the command to apply or un-apply this patch
I am a little bit annoyed because I think we are polluting this thread with other concerns

I am a little bit annoyed because I think we are polluting this thread with other concerns

Totally right :) ;)

@rgpublic after done an update - Upgrading drupal/core (9.5.3 => 9.5.7) and disable settings.local before, it seem to work without 500 errors on known pages. Your tips seem to be the real fix :)

You should have used it like so https://git.drupalcode.org/project/drupal/-/merge_requests/1943/diffs.patch

Preferably download it and use it as a local patch in your composer.json since, if the code in the pr changes, the patch will change as well with your next composer install/update whatever, and that might be something you don't want to happen uncontrollably.

I uploaded a patch you can use, and it will also be useful (probably) for allowing drupal.org test suite to run.

Thanks Marios, but I don't understand anymore. Some says patch isn't a good way, prefer using settings.php only, others say patch is needed. :/

Great discussion about patches, but let's please, out of respect for the 87 people following this issue, stop talking about patches here, and do not even reply to say "I agree" or "I understand" or something. We're done talking about patches! Thank you! Direct any further patch discussion here:

https://www.drupal.org/docs/develop/git/using-git-to-contribute-to-drupa... →
https://www.drupal.org/node/1399218/discuss →

Regarding settings.local.php and development.services.yml (which, rgpublic is correct, should not be enabled on production environments!), please refer to the files themselves and you can discuss it on the "Disable caching" page:

https://git.drupalcode.org/project/drupal/-/blob/10.1.x/sites/example.se...
https://git.drupalcode.org/project/drupal/-/blob/10.1.x/sites/developmen...
https://www.drupal.org/docs/develop/development-tools/disable-caching →
https://www.drupal.org/node/3340861/discuss →

You can also always go to Forums, drupal.stackexchange.com, Slack to get answers to your questions:

https://www.drupal.org/community/contributor-guide/reference-information... →

My next comment here is going to summarize the state of this ticket and hopefully get us back on track.

The most recent MR/patch is still mxr576's, !1943, from 8 March 2022.

https://git.drupalcode.org/project/drupal/-/merge_requests/1943

It is reviewed by the community (#62, #63) but it needs tests (#64).

Hi, I recently upgraded my site to D9.5 and I'm getting this error on some of my pages. So far I tried the following:
1) Created a patch from 1943 MR and applied as a local patch
2) Updated the `http.response.debug_cacheability_headers` to `FALSE`

None of these above works for me. The errors I see apart from the WSOD can be found below:

web_1  | [Wed Apr 12 07:08:19.863755 2023] [proxy_fcgi:error] [pid 9:tid 281473321831168] [client 172.23.0.2:34096] Premature end of script headers: index.php, referer: http://mysite.docksal.site/admin/content
web_1  | [Wed Apr 12 07:08:19.864173 2023] [proxy_fcgi:error] [pid 9:tid 281473321831168] [client 172.23.0.2:34096] AH01070: Error parsing script headers, referer: http://mysite.docksal.site/admin/content
web_1  | [Wed Apr 12 07:08:19.864194 2023] [proxy_fcgi:error] [pid 9:tid 281473321831168] (22)Invalid argument: [client 172.23.0.2:34096] AH01075: Error dispatching request to : , referer: http://mysite.docksal.site/admin/content
web_1  | 172.23.0.2 - - [12/Apr/2023:07:08:00 +0000] "GET /node/1901/edit?destination=/admin/content HTTP/1.1" 500 -

Any idea what else I can do ? Thanks in advance.

@dynamdilshan This ticket deals with limiting the X-Drupal-Cache-Tags and X-Drupal-Cache-Contexts headers. With http.response.debug_cacheability_headers set to false, these should not be an issue anyway. You can use xdebug or other debugging tools to step through the code in the patch and confirm whether or not those headers are limited or even need limiting.

I will note that other headers (including from the fastly and pantheon_advanced_page_cache modules) can get out of control and the patch doesn't account for them (though you could maybe argue it should). That's based on recent comments on this Docksal issue:

https://github.com/docksal/docksal/issues/1110#issuecomment-1216488250

Thanks @charginghawk for your reply. Due to some reason this patch and the http.response.debug_cacheability_headers set to false not working for me. I put this below code on index.php to debug and get the size of my headers

if ($config['config_split.config_split.local']['status']) {
  $response->headers->set('x-drupal-cache-tags', '');
  $response->headers->set('cache-tags', '');
}

And currently nuking all the cache tags on local is the only way for me to get the page working. I found found out some of my pages giving more than 8k size headers.

I was having a similar issue but it turned out not to be an issue with core. My enormous header was edge-cache-tags from the akamai module. Disabling the purge and akamai modules on local took care of it.

Here's a patch version of MR 1943 (latest commit bbcb4ac7), if anyone else needs it.
PS: our setup enforces static patches and MR diffs are not allowed (as they can change unexpectedly).

I was able to reduce the number of tags dramatically by updating the cache tag blacklist with all the options listed here: https://www.drupal.org/docs/contributed-modules/akamai/cache-tag-purging →

On some pages it reduced the number of tags from ~600 to ~100. Still a lot, but definitely an improvement.

Thank you, this fixed a problem in a project that was recently updated to Drupal 10 where I was suddenly unable to edit the homepage.
I could revert the revision of the homepage and then edit it. But after a while editing would again not work and again the /node/xxxx/edit page would return an error 500. This patch (the diff from the issue fork) fixes it in my D10 project.

Same here after update of project to 10.1.7
Updated the `http.response.debug_cacheability_headers` to `FALSE`
+ patch #87 fixed it

I tried to clarify current state in the issue body: we need test to move this to RTBC.

The MR should be used, ignore patches.

The patch is working for me in a real project.

#87 works on D9.5.11 core .

Ran into this recently loading the fields settings page for a content type after http.response.debug_cacheability_headers was set to "true" via the development.services.yml file; the site runs on ddev, Safari showed the site giving a 500 error and nothing I could do would give me an explanation of what was going on.

Considering this is still an issue in D11, I figured we should move it forward. Added tests and new MR for 11.x, back to review

smustgrave → changed the visibility of the branch 11.x to hidden.

Added some nitpicky change for the tests.

Leaving in review for additional eyes.

Changes loosk good to me. However, it would be great if we can run the tests only without the patch to confirm the test works ok (fails when no fix has been applied, passes when it is applied). I tried to run the "Test-only changes" stage at https://git.drupalcode.org/issue/drupal-2844620/-/jobs/1342592 but I says:

This job does not run automatically and must be started manually, but you do not have access to it.

@tunic this was previously run, but I guess retriggered on rebase. For access I think you just need to request access to the fork and then sign in to gitlab.

Either way, here's the pass on main, and fail on test-only run: https://git.drupalcode.org/issue/drupal-2844620/-/pipelines/148275

Ah, of course, I forgot to ask for access rights.

Issue is addressed, code issues have been solved, test-only patch fails as it should and tests passes with patch. I think this can be moved to RTBC but I'll wait for more reviewers.

Updating solution.

BTW, do we need a change record

Given this is a debug-specific change, I don't think a change record here is necessary; however, updating any docs referring to debugging headers is definitely a good idea.

Other doc page need also to be update https://www.drupal.org/docs/drupal-apis/cache-api/cache-tags-varnish → , https://www.drupal.org/project/purge → , https://www.drupal.org/project/varnish_purge →

#114, I think those are related but different headers. In this issue we are handling the headers X-Drupal-Cache-Tags and X-Drupal-Cache-Contexts that are sent by Drupal core when the cacheability debug functionality is on (http.response.debug_cacheability_headers: true). However, the links you provide refer to the Cache-Tags and Purge-Cache-Tags headers (each purger module uses one or the other).

While the information they carry is similar (the cache tag information) and all are affected by the same problem (header too large), the purger headers are not affected by the fix proposed here because that later headers are added by each purger module itself.

It can be interesting to propose a similar fix on the purger modules; however, it will probably be harder for Varnish to process a single header or multiple headers with an incremental sequence (Cache-Tags-1, Cache-Tags-2, etc) so I guess this is something that must be discussed.

ok, sorry for the confusion (TL; DR 😬).
I think purge should just split tags by 16k chunks and make multiple calls to Varnish/CDN.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. 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.