Tasks to fix and improve caching

[Toflar]

As pointed out in other issues already, the caching system still needs a lot of love. We are making good progress but there are still a lot of things to do to make Contao become the CMS with the best native caching support out there. We should not forget that our requirements are very complex and it will remain an ongoing topic that will accompany us for another few versions of Contao.
This issue should serve as a central place to keep track of all the TODO's around the HTTP cache.

Bugfixes

• For Contao versions 4.4 and 4.5, we must force public responses even though the session was started (see [RTM] Do not make the response private when saving the session #1388). @leofeyer

• The Symfony ResponseCacheStrategy does not differentiate between private responses and no-cache cache headers. We must provide a PR to fix that. ([HttpKernel] Correctly merging cache directives in HttpCache/ResponseCacheStrategy symfony/symfony#26532) @aschempp

• Our InsertTagsController must set max-age (client cache) settings based on page settings for BC insert tags (see Make sure insert tags get the same client cache time as the page #1390). @Toflar

• FrontendTemplate::addCacheHeadersToResponse() must be fixed because it only sets private. The no-cache stuff is missing. @leofeyer

Features

• We must ensure, we're not starting the session if we don't need it. Starting the session implicitly means that a response is private and may not be cached as it may contain personal information. As long as the core still requires stuff like this, we need to force public responses.

  • $_SESSION['FORM_DATA'] is a real killer here because we never unset the information stored in that value. We must introduce a config option in the form settings to disable that (or enable explicitly). It should be removed in version 5.

  • hasPreviousSession() should be added everywhere we access the session to make sure the session is not started if there's no cookie (Input class) (Only start the session if needed to find data in FORM_DATA #1471) @Toflar

• We need insert tags as fragments so every insert tag can decide for its own, how it would like to be rendered (and set the correct caching headers) ([RFC] InsertTags as fragments #1470) @Toflar

• Add support for tagging responses so that we can cache stuff "forever" and invalidate using cache tags. (Use toflar/psr6-symfony-http-cache-store manager-bundle#60) @Toflar

[leofeyer]

@contao/developers I need your feedback in order to fix the last open bugfix. These are the headers we used to send:

if (!headers_sent())
{
	if ($intCache > 0 && (\Config::get('cacheMode') == 'both' || \Config::get('cacheMode') == 'browser'))
	{
		header('Cache-Control: private, max-age=' . ($intCache - time()));
		header('Last-Modified: ' . gmdate('D, d M Y H:i:s', time()) . ' GMT');
		header('Expires: ' . gmdate('D, d M Y H:i:s', $intCache) . ' GMT');
	}
	else
	{
		header('Cache-Control: no-store, no-cache, must-revalidate, post-check=0, pre-check=0');
		header('Last-Modified: ' . gmdate('D, d M Y H:i:s') . ' GMT');
		header('Expires: Fri, 06 Jun 1975 15:10:00 GMT');
	}
}

See https://github.com/contao/core-bundle/pull/576/files#diff-9d1eb5ea5f99817ee33995c01eced477L320

@Toflar Your initial PR only uses setPrivate(), setMaxAge() and setSharedMaxAge(). Which headers do you want me to add?

[Toflar]

Only these, the rest is irrelevant for what you can configure in the page settings (Last-Modified is less-accurate and a fallback method for working with E-Tag which we don't do either). And Expires is harder to set than max-age but does the same. So we only need

• Private if client cache only
• no-cache, no-store for client and server cache in case it was disabled
• max-age value for client cache
• s-max-age for server cache

[leofeyer]

Hm, I still don't know exactly what to do. 😄 This is the current code:

global $objPage;

if (($objPage->cache === false || $objPage->cache === 0) && ($objPage->clientCache === false || $objPage->clientCache === 0))
{
	return $response->setPrivate();
}

// Do not cache the response if a user is logged in or the page is protected
// TODO: Add support for proxies so they can vary on member context
if (FE_USER_LOGGED_IN === true || BE_USER_LOGGED_IN === true || $objPage->protected || $this->hasAuthenticatedBackendUser())
{
	return $response->setPrivate();
}

if ($objPage->clientCache > 0)
{
	$response->setMaxAge($objPage->clientCache);
}

if ($objPage->cache > 0)
{
	$response->setSharedMaxAge($objPage->cache);
}

return $response;

The third and fourth if conditions are clear. Do I apply no-cache, no-store in both the first and second if condition?

And what is the difference between $objPage->cache === false and $objPage->cache === 0?

[Toflar]

And what is the difference between $objPage->cache === false and $objPage->cache === 0?

None.

Do I apply no-cache, no-store in both the first and second if condition?

Jop.

[leofeyer]

[leofeyer]

So then the first two if conditions are

@@ -376,6 +376,9 @@ class FrontendTemplate extends \Template
 
 		if (($objPage->cache === false || $objPage->cache === 0) && ($objPage->clientCache === false || $objPage->clientCache === 0))
 		{
+			$response->headers->addCacheControlDirective('no-cache');
+			$response->headers->addCacheControlDirective('no-store');
+
 			return $response->setPrivate();
 		}
 
@@ -383,6 +386,9 @@ class FrontendTemplate extends \Template
 		// TODO: Add support for proxies so they can vary on member context
 		if (FE_USER_LOGGED_IN === true || BE_USER_LOGGED_IN === true || $objPage->protected || $this->hasAuthenticatedBackendUser())
 		{
+			$response->headers->addCacheControlDirective('no-cache');
+			$response->headers->addCacheControlDirective('no-store');
+
 			return $response->setPrivate();
 		}

?

[aschempp]

Well private does not make sense if it is no-cache, right?

[Toflar]

It does. It should always be there, no matter the cache directive. They are semantically different and do not provide an answer to the same question.

[Toflar]

I've created two more PR's. That list is now reduced two one open item (the checkbox in the form settings). @leofeyer are you going to take over that one?

[leofeyer]

Yes.

[leofeyer]

@Toflar How does a form setting solve our problem again?

First we would have to add a third argument $blnNoSession to all Input::post*() methods. Then we would have to set this argument to true in the Form class.

1. What about the Widget::getPost() method?
2. What if someone uses the Input class without the Form class?

[aschempp]

I don't think that's the case. We just don't write to $_SESSION['FORM_DATA'] when submitting a form. Everything else solves automatically because no session will be started in that case, so neither Widget nor Input will try to use these values.

[leofeyer]

But Input::findPost() will still execute isset($_SESSION['FORM_DATA'][$strKey]) which will start the session, won't it?

[aschempp]

Not with #1471

[Toflar]

What about the Widget::getPost() method?

I guess this is the reason why this session stuff was introduced back 10 years ago. If you submitted the form with errors, I guess thanks to the session the form was not cleared and the values were propagated again so you could only fix what was wrong: I'm not sure at all though but I think it should not rely on the session either. Nowadays we should leverage browser capabilities to do so (if they do not automatically already, otherwise use localstorage or whatever).

What if someone uses the Input class without the Form class?

Then the session is getting started if the cookie is there. We cannot prevent that because of BC reasons. I'd really love to drop it completely but we can't.

[aschempp]

A submitted form is a POST request, there is no need to keep the session data to reload the value. There is only a reload after successful submit.

[Toflar]

I think that used to be different in earlier versions. That's maybe why.

[leofeyer]

Since the last open issue "insert tags as fragments" is up for discussion again (see contao/contao#555), I think we can eventually close this ticket.