Merge pull request #516 from FriendsOfSymfony/without-session

do not leak the Symfony-Session-NoAutoCacheControl header when the Symfony session system is not enabled.

Author: dbu

CHANGELOG.md

@@ -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
 -----
 

src/DependencyInjection/Compiler/SessionListenerRemovePass.php renamed to src/DependencyInjection/Compiler/SessionListenerPass.php

@@ -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}
@@ -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');
  }
 }

src/EventListener/UserContextListener.php

@@ -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
  */
@@ -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([
@@ -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, '<=')) {
  // header to avoid Symfony SessionListener overwriting the response to private
  $response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 1);
  }
@@ -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);
  }

src/FOSHttpCacheBundle.php

@@ -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;
@@ -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

src/Resources/config/user_context.xml

@@ -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>
 

tests/Unit/EventListener/UserContextListenerTest.php

@@ -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');
  }
 
@@ -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');
  }
 
@@ -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');
  }
 
@@ -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');
  }