Skip to content

Commit

Permalink
feature #26681 Allow to easily ask Symfony not to set a response to p…
Browse files Browse the repository at this point in the history
…rivate automatically (Toflar)

This PR was squashed before being merged into the 4.1-dev branch (closes #26681).

Discussion
----------

Allow to easily ask Symfony not to set a response to private automatically

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

This PR is related to the many discussions going on about the latest (Symfony 3.4+) changes regarding the way Symfony handles the session. I think we're almost there now, Symfony 4.1 automatically turns responses into `private` responses once the session is started and it's all done in the `AbstractSessionListener` where the session is also saved.
In other issues/PRs (#25583, #25699, #24988) it was agreed that setting the response to `private` if the session is started is a good default for Symfony. It was also agreed that setting it to `private` does not always make sense because you **can share a response** across sessions, it just requires a more complex caching setup with shared user context etc.
So there must be an easy way to disable this behaviour. Right now it's very hard to do so because what you end up doing is basically decorating the `session_listener` which is very hard because you have to keep track on that over different Symfony versions as the base listener might get additional features etc.

The [FOSCacheBundle](FriendsOfSymfony/FOSHttpCacheBundle#438) is already having this problem, [Contao](contao/core-bundle#1388) has the same issue and there will be probably more. Basically everyone that wants to share a response cache across the session will have to decorate the default listener. That's just too hard, so I came up with this solution. The header is easy. Every project can add that easily. It does not require any extension, configuration or adjustment of any service. It's clean, transparent and has absolutely no impact on "default" Symfony setups.

Would be happy to have some feedback before 4.1 freeze.

Commits
-------

0f36710 Allow to easily ask Symfony not to set a response to private automatically
  • Loading branch information
fabpot committed Mar 30, 2018
2 parents 6f15386 + 0f36710 commit f62a6d7
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 6 deletions.
Expand Up @@ -20,13 +20,22 @@
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* Sets the session in the request.
* Sets the session onto the request on the "kernel.request" event and saves
* it on the "kernel.response" event.
*
* In addition, if the session has been started it overrides the Cache-Control
* header in such a way that all caching is disabled in that case.
* If you have a scenario where caching responses with session information in
* them makes sense, you can disable this behaviour by setting the header
* AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER on the response.
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
* @author Tobias Schultze <http://tobion.de>
*/
abstract class AbstractSessionListener implements EventSubscriberInterface
{
const NO_AUTO_CACHE_CONTROL_HEADER = 'Symfony-Session-NoAutoCacheControl';

protected $container;

public function __construct(ContainerInterface $container = null)
Expand Down Expand Up @@ -60,13 +69,20 @@ public function onKernelResponse(FilterResponseEvent $event)
return;
}

$response = $event->getResponse();

if ($session->isStarted() || ($session instanceof Session && $session->hasBeenStarted())) {
$event->getResponse()
->setPrivate()
->setMaxAge(0)
->headers->addCacheControlDirective('must-revalidate');
if (!$response->headers->has(self::NO_AUTO_CACHE_CONTROL_HEADER)) {
$response
->setPrivate()
->setMaxAge(0)
->headers->addCacheControlDirective('must-revalidate');
}
}

// Always remove the internal header if present
$response->headers->remove(self::NO_AUTO_CACHE_CONTROL_HEADER);

if ($session->isStarted()) {
/*
* Saves the session, in case it is still open, before sending the response/headers.
Expand Down
Expand Up @@ -56,7 +56,7 @@ public function testSessionIsSet()
$this->assertSame($session, $request->getSession());
}

public function testResponseIsPrivate()
public function testResponseIsPrivateIfSessionStarted()
{
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
$session->expects($this->exactly(2))->method('isStarted')->willReturn(false);
Expand All @@ -74,6 +74,31 @@ public function testResponseIsPrivate()
$this->assertTrue($response->headers->hasCacheControlDirective('private'));
$this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
$this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
$this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
}

public function testResponseIsStillPublicIfSessionStartedAndHeaderPresent()
{
$session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
$session->expects($this->exactly(2))->method('isStarted')->willReturn(false);
$session->expects($this->once())->method('hasBeenStarted')->willReturn(true);

$container = new Container();
$container->set('initialized_session', $session);

$listener = new SessionListener($container);
$kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();

$response = new Response();
$response->setSharedMaxAge(60);
$response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 'true');
$listener->onKernelResponse(new FilterResponseEvent($kernel, new Request(), HttpKernelInterface::MASTER_REQUEST, $response));

$this->assertTrue($response->headers->hasCacheControlDirective('public'));
$this->assertFalse($response->headers->hasCacheControlDirective('private'));
$this->assertFalse($response->headers->hasCacheControlDirective('must-revalidate'));
$this->assertSame('60', $response->headers->getCacheControlDirective('s-maxage'));
$this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
}

public function testUninitilizedSession()
Expand Down

0 comments on commit f62a6d7

Please sign in to comment.