[3.4] ESI http_cache seems not written to cache folder anymore

[stefandoorn]

Symfony version(s) affected: 3.4.11 / 3.4.x-dev

Description

Today I upgraded 3.4.2 to 3.4.11 and the site went into degraded performance on production. It seems after local debugging that the ESI HTTP Cache is not being written (which I didn't test locally before deployment). Local testing with the setup as described below.

Rendering done using render_esi. On 3.4.2, the var/cache/dev_performance/http_cache folder gets populated with folders & files. On 3.4.11 this does not happen (it stays empty).

I just installed 3.4.x-dev to verify it in there, and it also does not work:

 - Installing symfony/symfony (3.4.x-dev 0e0cf14): Cloning 0e0cf1467a from cache

How to reproduce

framework:
    fragments: { path: /_fragment }
    esi: { enabled: true }

$kernel = new AppKernel('dev_performance', false);
$kernel = new AppCache($kernel);

        {{ render_esi(url('esi_test')) }}

Initially I thought this PR might fix it (#27467), but that one should be included in the commit I'm testing I think.

[nicolas-grekas]

Can you provide a working reproducer, e.g. a repo we could clone and run?

[stefandoorn]

Tried to reproduce it on a clean installation, and it seems to work on 3.4.11 also. So probably it's something application specific.. might become hard to figure out what that exactly is. Will try to upgrade 3.4.2 -> 3.4.3 etc. to see where my issue gets introduced.

[stefandoorn]

Request to http://app.local/app_dev_performance.php. Then:

$ ls -lah /dev/shm/sylius/cache/dev_performance/http_cache/
total 0
drwxr-xr-x.  4 vagrant vagrant  80 Jun 22 16:12 ./
drwxr-xr-x. 11 vagrant vagrant 360 Jun 22 16:12 ../
drwxr-xr-x. 10 vagrant vagrant 200 Jun 22 16:12 en/
drwxr-xr-x. 12 vagrant vagrant 240 Jun 22 16:12 md/

$ rm -rf /dev/shm/sylius/*

$ composer require symfony/symfony:3.4.3 --no-scripts
./composer.json has been updated                                                  
Loading composer repositories with package information
Updating dependencies (including require-dev)                                     
Package operations: 0 installs, 1 update, 0 removals
  - Updating symfony/symfony (v3.4.2 => v3.4.3):  Checking out 21abeae69b
Writing lock file
Generating autoload files

Again a request to http://app.local/app_dev_performance.php. Then:

$ ls -lah /dev/shm/sylius/cache/dev_performance/http_cache/
total 0
drwxr-xr-x.  2 vagrant vagrant  40 Jun 22 16:15 ./
drwxr-xr-x. 11 vagrant vagrant 360 Jun 22 16:16 ../

[stefandoorn]

Narrowed it down to a change in Symfony v3.4.3, if I revert the change from this PR: https://github.com/symfony/symfony/pull/25583/files

I get to:

$ ls -lah /dev/shm/sylius/cache/dev_performance/http_cache/
total 0
drwxr-xr-x.  4 vagrant vagrant  80 Jun 22 16:28 ./
drwxr-xr-x. 11 vagrant vagrant 360 Jun 22 16:16 ../
drwxr-xr-x. 10 vagrant vagrant 200 Jun 22 16:28 en/
drwxr-xr-x. 12 vagrant vagrant 240 Jun 22 16:28 md/

[stefandoorn]

It makes sense that the clean installation didn't produce me this result, as in there no sessions are involved. In this project each page has a session (cart) available, although the chunks I'm loading are generated independent from sessions - but as it's using the framework I think it still has sessions involved.

[nicolas-grekas]

So, that's on you then, and the previous behavior was dangerous, as personal state could leak in the cached fragments!

[stefandoorn]

Yeah, good one.

But, what about backporting this PR to 3.4? https://github.com/symfony/symfony/pull/26681/files. Seems to me like a proper solution in which I can define from the code context when to cache, defaulting to no cache when a session is involved.

[nicolas-grekas]

Because that's a new feature. If you really want to cache a session-enabled fragment, think twice, then register a late listener to mimic what this PR will provide?

[emodric]

Or you can just decorate the listener to disable setting the response to private, similar to what FOS HTTP Cache bundle ia doing: FriendsOfSymfony/FOSHttpCacheBundle#435

[stefandoorn]

Thanks @nicolas-grekas & @emodric :) Found a solution that works for now, and working on improving it for the future!