Issue #3620917: ESI fragments are never cached, and they lose the cache tags of the content they show
Fixes three faults that hide each other. See the issue summary for the full analysis.
The VCL refused every ESI fragment. The callbacks are declared with no_cache, so Drupal marks fragments private, and the rule meant to let those through checks for X-DOESI, which a fragment never carries by design. Excepting the ESI path is not enough on its own: vcl_backend_response did not end in a return, so the built in rules ran afterwards and refused the response again. Returning unconditionally is not an option either, since an authenticated response carries X-DOESI once ESI is on, and a redirect meant for one visitor would land in a bin shared by everyone with the same roles.
So the exception names one route, /adv_varnish/esi/block/, no-store is refused ahead of it, and the early return covers a plain 200 on that route alone. The status matters: the module puts its own lifetime on anything that is not an ESIResponse, so an error there would otherwise be held for the full TTL. The other route, user_blocks, carries the personal data that has to be fresh on every page and is still refused.
Fragments carried none of the tags of what they show. ESIBlockController rendered with renderInIsolation(), which keeps the metadata out of the surrounding render context, and never read it back off the build. The metadata is now taken from the build after rendering.
A stale bin cookie turned a fragment into a redirect. That is what [#3072725] addressed for the page, and it was still reachable on the callbacks. The reload is skipped for the ESI callbacks now; the cookie is still set, and the request for the page itself carries out the reload.
Tests
A VTC test with seven cacheability cases, two functional tests for the headers Drupal has to send for the VCL to work, and two unit tests for the reload rule. Every one was checked to fail without the change it covers.
Green on Drupal 10, 11 and 12 (125 tests), 13 VTC tests, and verified against a live Varnish: a fragment goes MISS then HIT, saving a node invalidates it, two users each get their own copy, and user_blocks is never held.
For the release notes
Today a wrong cache granularity on a block has no effect, because nothing is cached. After this change a block holding personal data marked "Shared" will be served to other visitors. The fix turns a dormant misconfiguration into a live leak, and that needs saying out loud.
varnish/default.vcl is a sample. Sites running their own adapted VCL get the tags from this change but no caching until they apply the VCL part by hand. Suggest releasing this as 4.2.0 rather than a patch, so the notes get read.