Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Changelog
=========

2.6.1
-----

### Fixed

* Do not leak the `Symfony-Session-NoAutoCacheControl` header when the Symfony session system is not enabled.

2.6.0
-----

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* Remove the session listener decorator again if the application has no session listener.
* Undo our workarounds for the Symfony session listener when the session
* system of Symfony has not been activated.
*
* This will happen on some APIs when the session system is not activated.
* - Set the hasSessionListener option of the UserContextListener to false to
* avoid the AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER being
* leaked to clients.
* - Remove the session listener decorator we configured.
*/
class SessionListenerRemovePass implements CompilerPassInterface
class SessionListenerPass implements CompilerPassInterface
{
/**
* {@inheritdoc}
Expand All @@ -30,6 +34,10 @@ public function process(ContainerBuilder $container)
return;
}

if ($container->hasDefinition('fos_http_cache.event_listener.user_context')) {
$contextListener = $container->getDefinition('fos_http_cache.event_listener.user_context');
$contextListener->setArgument(5, false);
}
$container->removeDefinition('fos_http_cache.user_context.session_listener');
}
}
18 changes: 14 additions & 4 deletions src/EventListener/UserContextListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ class UserContextListener implements EventSubscriberInterface
*/
private $options;

/**
* Whether the application has a session listener and therefore could
* require the AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER.
*
* @var bool
*/
private $hasSessionListener;

/**
* @var string
*/
Expand All @@ -78,12 +86,14 @@ public function __construct(
HashGenerator $hashGenerator,
RequestMatcherInterface $anonymousRequestMatcher = null,
ResponseTagger $responseTagger = null,
array $options = []
array $options = [],
bool $hasSessionListener = true
) {
$this->requestMatcher = $requestMatcher;
$this->hashGenerator = $hashGenerator;
$this->responseTagger = $responseTagger;
$this->anonymousRequestMatcher = $anonymousRequestMatcher;
$this->responseTagger = $responseTagger;
$this->hasSessionListener = $hasSessionListener;

$resolver = new OptionsResolver();
$resolver->setDefaults([
Expand Down Expand Up @@ -146,7 +156,7 @@ public function onKernelRequest(GetResponseEvent $event)
$response->setClientTtl($this->options['ttl']);
$response->setVary($this->options['user_identifier_headers']);
$response->setPublic();
if (4 <= Kernel::MAJOR_VERSION && 1 <= Kernel::MINOR_VERSION) {
if ($this->hasSessionListener && version_compare('4.1', Kernel::VERSION, '<=')) {
Copy link
Contributor Author

@dbu dbu Mar 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the old code would have been broken with symfony 5.0.
<= comparator seems to work with all php versions: https://3v4l.org/GCltV

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old code failed with 5.0, but works with 5.1: https://3v4l.org/PtroA

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly. thanks for looking at this, reminds me that i should merge it.

// header to avoid Symfony SessionListener overwriting the response to private
$response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 1);
}
Expand Down Expand Up @@ -210,7 +220,7 @@ public function onKernelResponse(FilterResponseEvent $event)
}

// For Symfony 4.1+ if user hash header was in vary or just added here by "add_vary_on_hash"
if (4 <= Kernel::MAJOR_VERSION && 1 <= Kernel::MINOR_VERSION && in_array($this->options['user_hash_header'], $vary)) {
if ($this->hasSessionListener && \version_compare('4.1', Kernel::VERSION, '<=') && in_array($this->options['user_hash_header'], $vary)) {
// header to avoid Symfony SessionListener overwriting the response to private
$response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 1);
}
Expand Down
4 changes: 2 additions & 2 deletions src/FOSHttpCacheBundle.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use FOS\HttpCache\UserContext\ContextProvider;
use FOS\HttpCacheBundle\DependencyInjection\Compiler\HashGeneratorPass;
use FOS\HttpCacheBundle\DependencyInjection\Compiler\LoggerPass;
use FOS\HttpCacheBundle\DependencyInjection\Compiler\SessionListenerRemovePass;
use FOS\HttpCacheBundle\DependencyInjection\Compiler\SessionListenerPass;
use FOS\HttpCacheBundle\DependencyInjection\Compiler\TagListenerPass;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\HttpKernel\Bundle\Bundle;
Expand All @@ -33,7 +33,7 @@ public function build(ContainerBuilder $container)
if (version_compare(Kernel::VERSION, '3.4', '>=')
&& version_compare(Kernel::VERSION, '4.1', '<')
) {
$container->addCompilerPass(new SessionListenerRemovePass());
$container->addCompilerPass(new SessionListenerPass());
}

// Symfony 3.3 and higher
Expand Down
1 change: 1 addition & 0 deletions src/Resources/config/user_context.xml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<argument type="service" id="fos_http_cache.user_context.anonymous_request_matcher" />
<argument type="service" id="fos_http_cache.http.symfony_response_tagger" on-invalid="ignore" />
<argument>%fos_http_cache.event_listener.user_context.options%</argument>
<argument>true</argument>
<tag name="kernel.event_subscriber" />
</service>

Expand Down
36 changes: 32 additions & 4 deletions tests/Unit/EventListener/UserContextListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public function testOnKernelResponse()

public function testOnKernelResponseSetsNoAutoCacheHeader()
{
if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
if (\version_compare('4.1', Kernel::VERSION, '>')) {
$this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
}

Expand All @@ -225,9 +225,37 @@ public function testOnKernelResponseSetsNoAutoCacheHeader()
$this->assertEquals(1, $event->getResponse()->headers->get(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
}

public function testOnKernelResponseDoesNotSetNoAutoCacheHeaderWhenNoSessionListener()
{
if (\version_compare('4.1', Kernel::VERSION, '>')) {
$this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
}

$request = new Request();
$request->setMethod('HEAD');
$request->headers->set('X-User-Context-Hash', 'hash');

$hashGenerator = \Mockery::mock(HashGenerator::class);

$userContextListener = new UserContextListener(
$this->getRequestMatcher($request, false),
$hashGenerator,
null,
null,
[],
false
);
$event = $this->getKernelResponseEvent($request);

$userContextListener->onKernelResponse($event);

$this->assertContains('X-User-Context-Hash', $event->getResponse()->headers->get('Vary'));
$this->assertFalse($event->getResponse()->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
}

public function testOnKernelResponseSetsNoAutoCacheHeaderWhenCustomHeader()
{
if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
if (\version_compare('4.1', Kernel::VERSION, '>')) {
$this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
}

Expand All @@ -250,7 +278,7 @@ public function testOnKernelResponseSetsNoAutoCacheHeaderWhenCustomHeader()

public function testOnKernelResponseSetsNoAutoCacheHeaderWhenCustomHeaderAndNoAddVaryOnHash()
{
if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
if (\version_compare('4.1', Kernel::VERSION, '>')) {
$this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
}

Expand Down Expand Up @@ -278,7 +306,7 @@ public function testOnKernelResponseSetsNoAutoCacheHeaderWhenCustomHeaderAndNoAd

public function testOnKernelResponseDoesNotSetNoAutoCacheHeaderWhenNoCustomHeaderAndNoAddVaryOnHash()
{
if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
if (\version_compare('4.1', Kernel::VERSION, '>')) {
$this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
}

Expand Down