http_cache not working #1246

Metis77 · 2017-12-18T22:03:20Z

after updating to 4.4.9 var/cache/prod/http_cache stopped working.
with 4.4.6 it still worked.

"Systemwartung" say 0 cached files and the folder http_cache stays empty.

tested in multiple websites, but same server.

bytehead · 2017-12-18T22:12:54Z

I've encountered the same.

bytehead · 2017-12-18T22:15:21Z

/cc @Toflar

Toflar · 2017-12-18T22:41:33Z

Can you test with 4.4.8 or does "with 4.4.6 it still worked" mean, that's the last one it worked with?

Metis77 · 2017-12-18T22:44:53Z

just tested:
4.4.9 - not working
4.4.8 - not working
4.4.7 - I dont know
4.4.6 - worked

Toflar · 2017-12-18T22:45:36Z

Ok, I'll let you find out if 4.4.7 works then.

Metis77 · 2017-12-18T22:58:10Z

well, updating from a Contao 4.4.4 version to 4.4.7 specifically failed using contao manager oO.
maybe another issue.

Without beeing able to install specific version with contao manager, I cant test right now.
Tomorrow I will give it another try.

Toflar · 2017-12-19T00:12:49Z

Okay after an hour of debugging, I found the issue. It was introduced by @aschempp in cae98ce#diff-6c590ef7e538a1f4a9115824540acb51.
I think merging the Cache-Control header needs special handling.

In my case, the $response correctly contained the Cache-Control header public, s-maxage=10800. In $this->headers the Cache-Control header contains Cache-Control: max-age=0, private, must-revalidate. The end result is max-age=0, must-revalidate, private, public, s-maxage=10800 which is considered to be private and thus nothing is cached.
I have no idea why the Cache-Control header is even part of headers_list(). I debugged using headers_list() with the two arguments to debug who set the header but the result was an empty string and 0 which tells me it's coming natively from the server.

There's certainly some kind of bug in Symfony because when merging the Cache-Control headers it shouldn't be private and public at the same time. But also I wonder if it ever makes sense to merge a cache header?

Toflar · 2017-12-19T00:17:24Z

It's wrong that public is still there but basically it does the right thing. It merges the private instruction from headers_list() onto the response which turns it private. I just have no clue why there's a default Cache-Control header present 😄

Toflar · 2017-12-19T00:30:25Z

It's not by default, it's set during the preflight request. For whatever reason. Nobody uses header() there? Man, this is fucked up as fuck. I need some sleep otherwise my brain is going to explode.

bytehead · 2017-12-19T05:43:02Z

Thanks for investigating. This covers my observations and I ended up quite confused 😄

aschempp · 2017-12-19T07:43:04Z

Doesn‘t our Template::getResponse method set everything to private if it‘s not public?

leofeyer · 2017-12-19T07:43:33Z

I cannot reproduce the issue on contao.org:

bin/console cache:clear

find var/cache/prod/http_cache -type f | wc -l
0

One minute later:

find var/cache/prod/http_cache -type f | wc -l
20

Toflar · 2017-12-19T08:01:26Z

Doesn‘t our Template::getResponse method set everything to private if it‘s not public?

As I said, it happens during the process of merging http headers. @leofeyer what stack do you use on contao.org?

leofeyer · 2017-12-19T08:31:55Z

Ubuntu, Nginx and PHP-FPM with PHP 7.1.

Metis77 · 2017-12-19T09:18:24Z

Ubuntu, Nginx and PHP-FPM with PHP 7.1.

same stack here, but with that error.

leofeyer · 2017-12-19T09:34:28Z

I'm not using the managed edition on contao.org. Maybe that makes the difference?

bytehead · 2017-12-19T09:35:12Z

I don't use the managed edition either, but still get the wrong behaviour.

Toflar · 2017-12-19T12:08:49Z

I think I‘ve found the issue. I‘d like to investigate a bit more on the reasons before I work on a solution. Will report back.

Toflar · 2017-12-19T15:29:48Z

Can you guys check your phpinfo() and tell me the session.cache_limiter setting. I suppose it contains nocache for all of you except for @leofeyer?

bytehead · 2017-12-19T15:40:50Z

$ php -i | grep cache_limiter
761:session.cache_limiter => nocache => nocache

leofeyer · 2017-12-19T16:23:12Z

Probably related: symfony/symfony-docs#8784

Toflar · 2017-12-19T18:53:44Z

Okay, so here's the summary of a day of debugging and investigation:

Why does the issue suddenly appear?

Normally, when you call session_start() to start the session, it directly sets additional headers to make sure the response is private. This makes a lot of sense because - in theory - whenever you start a session, the response is private to you by defintion.
Prior to Symfony 3.4 and 4.0, setting these headers automatically was suppressed. This was considered to be a bug and fixed in the latest versions. For the record: I support this decision, this is a bug and even if it changed the behavior, it is a correct bugfix.

What effect does it have on Contao?

As of now, everytime you work with the session. A response turns private and thus, the reverse proxy (or call it "html cache" if you like) never stores any response.
The problem we have is that we always work with the session. E.g. we store the REQUEST_TOKEN in the session, locale stuff, $_SESSION['FORM_DATA'] when submitting a form etc. pp. The list goes on forever.

So what now?

The correct solution would be to never work with the session if you don't need it. If nobody accesses the session of Symfony, the session is never started and thus that issue would not occur.
We improved that already in Contao 4.5 by having a separate cookie for the CSRF protection (Request Token) so it's not dependent on the session anymore. We would need further adjustments (LocaleListener for example) to drop usage of the session and we could eventually manage to make sure, the Contao Core (I don't even want to think about extensions) does not need any session in the front end anymore. However, what would that mean? Yes, exactly: this would cause that the session is never started. We cannot do this because that would be a huge BC break. Everyone accessing $_SESSION would always get null because nobody started the session. In fact, nobody noticed this but $_SESSION being present was a lucky side effect of having the RequestToken class accessing - and thus starting - the session for every request :D

So you see, it all comes down to this: In Contao, a session never implied a private response. This was wrong ever since. This is also why we keep telling users not to enable cache on certain pages (e.g. when there's a form on it). We have the same issue here again.
In Contao it was always the developers that controlled if a response is cacheable or not (by doing funny stuff like $objPage->cache = 0; or whatever).

So can we just restore the old behavior?

No. We should not adjust a Symfony-wide configuration. We could adjust the MergeHttpHeadersListener to just ignore Cache-Control headers. That would basically restore that behaviour for all Contao related routes but not affect other routes.

What would be the right solution?

Stop using $_SESSION! Stop using it, use the Symfony session ($request->getSession()) instead and always check if it was already started if you don't explicitly need it. And also make sure you understand how proxies work and if you really need a session. Do not just blindly store things in the session because - again - that kind of makes every response private and thus these responses can never be cached.
I think we could just adjust the MergeHttpHeadersListener but it kind of hides the real issues. We can just continue on and even when using the proper Symfony session object, nobody notices any issues with the cache because it's hidden by the MergeHttpHeadersListener. Remember: making all responses private when starting a session is done on purpose. There's a reason why you should not do otherwise. We just cannot quickly fix it because of BC. But maintaining BC does not mean we should not force our developers to use their brains and do things correctly 😄 In my opinion, we should maybe adjust the MergeHttpHeadersListener as a quick fix but I would prefer reverting that change later on, cleaning up everything in the Contao core (no more $_SESSION) and providing a BC layer for $_SESSION. We could set it to an ArrayAccess object that could start the session as soon as accessed. Similar to what we have already (but it would need to start the session in addition to just behaving like a bag):

$_SESSION['BE_DATA'] = $this->session->getBag('contao_backend');
$_SESSION['FE_DATA'] = $this->session->getBag('contao_frontend');

Not sure if this would work out but maybe worth a try. I just don't feel very comfortable just adjusting the listener and ignoring further issues.

Metis77 · 2017-12-19T19:24:45Z

Can you guys check your phpinfo() and tell me the session.cache_limiter setting. I suppose it contains nocache for all of you except for @leofeyer?

also: session.cache_limiter => nocache

Toflar · 2017-12-20T15:52:55Z

So providing a BC layer for $_SESSION does not work, I've tested that. The framework has to start the session for BC all the time 😢

leofeyer · 2017-12-20T15:54:06Z

This will only change in Contao 4.5.

Toflar · 2017-12-20T15:58:43Z

It's a bugfix that we can merge into 4.4, it doesn't change anything.

Toflar · 2017-12-20T16:03:52Z

#1260

Did not adjust tests yet but can you check if that fixes the issue?

Metis77 · 2017-12-20T21:19:51Z

@Toflar
not sure if this will help you:
after replacing those two files from #1260 in my Contao 4.4.9 installation, http_cache stays empty.
In Contao Manager I get following error on rebuild cache:

In ContaoFramework.php line 303:                                   
  Call to a member function getSession() on null

Toflar · 2018-02-13T11:01:00Z

Can you also check if the autologin functionality works for you? terminal42/header-replay-bundle#8 is the reason why 4.4.13 requires header-replay-bundle 1.3 but it works for me with all 1.4.* versions so there's no reason to me to stick with the old version :)

joeherold · 2018-02-13T11:07:04Z

@Toflar works, no problem so far with Contao 4.4.13 and "terminal42/header-replay-bundle": "dev-hotfix/1.4.2 as 1.3.2"

joeherold · 2018-02-13T14:20:10Z

@Toflar freue mich auf den Kaffee... :)

fenepedia · 2018-02-19T08:46:39Z

@joeherold @Toflar Ich spendier euch beiden den Kaffee… Meldet euch einfach in Salzburg bei mir (Christian von der Contao Academy)

Metis77 · 2018-02-27T17:17:07Z

Thanks for all the work on all the different tasks.

After updating it is still not working.
But the ticket is closed again, so it should work?

I updated with Contao Manager to Contao v4.4.14 and Symphony v3.4.4.

Did I miss anything?

  - Updating symfony/polyfill-mbstring (v1.6.0 => v1.7.0): Downloading (100%)
  - Updating symfony/polyfill-util (v1.6.0 => v1.7.0): Downloading (100%)
  - Updating symfony/polyfill-php70 (v1.6.0 => v1.7.0): Downloading (100%)
  - Updating symfony/polyfill-php56 (v1.6.0 => v1.7.0): Downloading (100%)
  - Updating symfony/symfony (v3.4.3 => v3.4.4): Downloading (100%)
  - Updating symfony/polyfill-intl-icu (v1.6.0 => v1.7.0): Downloading (100%)
  - Updating symfony/polyfill-apcu (v1.6.0 => v1.7.0): Downloading (100%)
  - Updating webmozart/assert (1.2.0 => 1.3.0): Downloading (100%)
  - Updating php-http/discovery (1.3.0 => 1.4.0): Downloading (100%)
  - Updating friendsofsymfony/http-cache (2.1.0 => 2.1.2): Downloading (100%)
  - Updating terminal42/header-replay-bundle (1.3.0 => 1.4.2): Downloading (100%)
  - Updating tecnickcom/tcpdf (6.2.13 => 6.2.17): Downloading (100%)
  - Updating matthiasmullie/path-converter (1.1.0 => 1.1.1): Downloading (100%)
  - Updating matthiasmullie/minify (1.3.58 => 1.3.59): Downloading (100%)
  - Updating contao-components/simplemodal (2.0.7 => 2.0.8): Downloading (100%)
  - Updating contao/core-bundle (4.4.13 => 4.4.14): Downloading (100%)
  - Updating contao/calendar-bundle (4.4.13 => 4.4.14): Downloading (100%)
  - Updating contao/faq-bundle (4.4.13 => 4.4.14): Downloading (100%)
  - Updating nelmio/security-bundle (2.4.0 => 2.5.0): Downloading (100%)
  - Updating lexik/maintenance-bundle (v2.1.3 => v2.1.5): Downloading (100%)
  - Updating contao/installation-bundle (4.4.13 => 4.4.14): Downloading (100%)
  - Updating contao/manager-bundle (4.4.13 => 4.4.14): Downloading (100%)
  - Updating contao/news-bundle (4.4.13 => 4.4.14): Downloading (100%)

joeherold · 2018-02-27T17:19:16Z

@Metis77 use this: https://packagist.org/packages/pliigo/pagecacheenabler-bundle

Metis77 · 2018-02-27T20:38:03Z

While I was hoping for a contao built in solution, pagecacheenabler-bundle workes for me.
Thank you for that!

I would still like to know, what the status of this issue is?

Is there any other open issue I should follow, to keep updated on this? Maybe something on the Symphony side?

Toflar · 2018-02-27T22:59:08Z

The issue is still open so why should it be fixed? There are a few fixes needed, they are all mentioned here: #1389

Metis77 · 2018-02-28T08:09:28Z

@Toflar great thanks!
I see, there is much to come.
The tagged cache thing sounds awesome!

leofeyer · 2018-03-20T08:30:19Z

The issue has eventually been fixed in Contao 4.4.15 and Contao 4.5.5.

iGweb · 2018-03-26T16:41:48Z

Hello,

I am on a 4.5.6 version of contao.
When I configure the cache to 60 minutes on my site (https://www.igweb.fr)
it's always 15 minutes away. (you can test here: https://www.giftofspeed.com/cache-checker/)

Do you have an idea of the problem ?

thank you in advance
Samuel

Toflar · 2018-03-26T16:49:16Z

Are you talking about the client (your browser) or the reverse proxy cache (var/cache/http_cache) time?

iGweb · 2018-03-26T17:06:37Z

client (your browser)

Toflar · 2018-03-26T18:47:38Z

Yeah, this is a Symfony bug, unrelated to Contao and still not fixed. We're working on fixing this together with other Symfony devs: symfony/symfony#26532

iGweb · 2018-03-27T07:24:39Z

Ah OK ! Thank you for that answer. I will wait a bit ... ;-)

Thank you

iGweb · 2018-03-28T10:21:15Z

Hello,

Is there a way around the problem while waiting for the fix?

Thank you

Toflar · 2018-03-28T10:44:09Z

Unfortunately, there's none.

iGweb · 2018-03-30T08:42:05Z

Hello,
is the use of "cloudflare" a good alternative to my problem? (https://www.cloudflare.com)

Thank you

Metis77 · 2018-03-30T10:07:38Z

@iGweb
You should not chain caches – so if using cloudflare, you should not use contao cache at all.

Cloudflare free plan caches for 2h on client side.
Cloudflare http cache needs to be enabled by PageRules – otherwise it will only cache assets.

Cloudflare below enterprise plan does not distinguish between browser user agents, like contao does.
That is not a problem, if you do not use that - for example for the CSS classes set on body tag.

iGweb · 2018-03-30T10:10:34Z

ok thanks for these precision :-)

aschempp · 2018-04-03T09:39:14Z

You should not chain caches – so if using cloudflare, you should not use contao cache at all.

Actually, Contao should allow you to bypass the local cache and still use the site configuration. See contao/manager-bundle#62

iGweb · 2018-07-03T22:08:16Z

Hello,

I have the same problem on one of my website: https://www.jalmalv85.fr
Contao: 4.4.20

To be able to access the site in HTTPS version I have to comment the following line in the file web/app.php:
// Enable the Symfony reverse proxy
//$kernel = new ContaoCache($kernel);
Request::enableHttpMethodParameterOverride();

Is a problem?
Is there a solution to correct this?

Thank you

aschempp · 2018-07-04T07:17:45Z

Is a problem?
Is there a solution to correct this?

The only problem is that the file will be overwritten on a Contao update. But contao/manager-bundle#71 has been merged so this feature will be available in Contao 4.6

iGweb · 2018-07-04T07:34:05Z

Ok Thank you

Description ----------- Fixes #1243 Commits ------- 3ded076e Fix possible null value 60836e49 Also check for null on json_decode

leofeyer added the bug label Dec 19, 2017

leofeyer added this to the 4.4.10 milestone Dec 19, 2017

joeherold mentioned this issue Feb 13, 2018

Fix http_cache of issue 1246 #1368

Closed

leofeyer closed this as completed Mar 20, 2018

leofeyer added this to the 4.4.15 milestone Mar 20, 2018

leofeyer modified the milestones: 4.4.15, 4.4 May 14, 2019

leofeyer pushed a commit that referenced this issue Jan 27, 2020

Fix possible null value access in BackupCodeManager (see #1246)

877aaf7

Description ----------- Fixes #1243 Commits ------- 3ded076e Fix possible null value 60836e49 Also check for null on json_decode

http_cache not working #1246

http_cache not working #1246

Comments

Metis77 commented Dec 18, 2017 • edited

bytehead commented Dec 18, 2017

bytehead commented Dec 18, 2017

Toflar commented Dec 18, 2017

Metis77 commented Dec 18, 2017

Toflar commented Dec 18, 2017

Metis77 commented Dec 18, 2017 • edited

Toflar commented Dec 19, 2017

Toflar commented Dec 19, 2017

Toflar commented Dec 19, 2017

bytehead commented Dec 19, 2017

aschempp commented Dec 19, 2017

leofeyer commented Dec 19, 2017

Toflar commented Dec 19, 2017

leofeyer commented Dec 19, 2017

Metis77 commented Dec 19, 2017 • edited

leofeyer commented Dec 19, 2017

bytehead commented Dec 19, 2017

Toflar commented Dec 19, 2017

Toflar commented Dec 19, 2017 • edited

bytehead commented Dec 19, 2017 • edited

leofeyer commented Dec 19, 2017

Toflar commented Dec 19, 2017

Why does the issue suddenly appear?

What effect does it have on Contao?

So what now?

So can we just restore the old behavior?

What would be the right solution?

Metis77 commented Dec 19, 2017

Toflar commented Dec 20, 2017

leofeyer commented Dec 20, 2017

Toflar commented Dec 20, 2017

Toflar commented Dec 20, 2017

Metis77 commented Dec 20, 2017

Toflar commented Feb 13, 2018

joeherold commented Feb 13, 2018

joeherold commented Feb 13, 2018

fenepedia commented Feb 19, 2018 • edited

Metis77 commented Feb 27, 2018

joeherold commented Feb 27, 2018

Metis77 commented Feb 27, 2018 • edited

Toflar commented Feb 27, 2018

Metis77 commented Feb 28, 2018

leofeyer commented Mar 20, 2018

iGweb commented Mar 26, 2018

Toflar commented Mar 26, 2018

iGweb commented Mar 26, 2018

Toflar commented Mar 26, 2018

iGweb commented Mar 27, 2018

iGweb commented Mar 28, 2018

Toflar commented Mar 28, 2018

iGweb commented Mar 30, 2018

Metis77 commented Mar 30, 2018

iGweb commented Mar 30, 2018

aschempp commented Apr 3, 2018

iGweb commented Jul 3, 2018

aschempp commented Jul 4, 2018

iGweb commented Jul 4, 2018

Metis77 commented Dec 18, 2017 •

edited

Metis77 commented Dec 18, 2017 •

edited

Metis77 commented Dec 19, 2017 •

edited

Toflar commented Dec 19, 2017 •

edited

bytehead commented Dec 19, 2017 •

edited

fenepedia commented Feb 19, 2018 •

edited

Metis77 commented Feb 27, 2018 •

edited