bug #36063 [FrameworkBundle] start session on flashbag injection (Wil…

…liam Arslett)

This PR was squashed before being merged into the 3.4 branch.

Discussion
----------

[FrameworkBundle] start session on flashbag injection

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix [#33084](#33084)
| License       | MIT

This PR addresses an issue whereby if the FlashBag is injected into the application using the default service configuration, we cannot rely that the session has been started. This behaviour is in contradiction to [the docs](https://symfony.com/doc/current/session.html#avoid-starting-sessions-for-anonymous-users):

> Sessions are automatically started whenever you read, write or even check for the existence of data in the session.

This is because symfony ensures the session has been started on calls to getFlashBag() which is normally how the flashbag will be accessed but this is not called if you inject the FlashBag directly into the container.

I have addressed this issue by changing the way the Flashbag service is built so that it uses Session as a factory service and getFlashBag as a factory method. This means that anywhere in symfony where FlashBag is injected can now rely on the fact the session is started.

I have also added a new functional test to verify this behaviour.

Commits
-------

e8b4d35 [FrameworkBundle] start session on flashbag injection

src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml

@@ -13,8 +13,6 @@
 
         <service id="session" class="Symfony\Component\HttpFoundation\Session\Session" public="true">
             <argument type="service" id="session.storage" />
-            <argument type="service" id="session.attribute_bag" />
-            <argument type="service" id="session.flash_bag" />
         </service>
 
         <service id="Symfony\Component\HttpFoundation\Session\SessionInterface" alias="session" />
@@ -37,10 +35,14 @@
             <argument type="service" id="session.storage.metadata_bag" />
         </service>
 
-        <service id="session.flash_bag" class="Symfony\Component\HttpFoundation\Session\Flash\FlashBag" />
+        <service id="session.flash_bag" class="Symfony\Component\HttpFoundation\Session\Flash\FlashBag">
+            <factory service="session" method="getFlashBag"/>
+        </service>
         <service id="Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface" alias="session.flash_bag" />
 
-        <service id="session.attribute_bag" class="Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag" />
+        <service id="session.attribute_bag" class="Symfony\Component\HttpFoundation\Session\Attribute\AttributeBag">
+            <factory service="session" method="getAttributeBag"/>
+        </service>
 
         <service id="session.storage.mock_file" class="Symfony\Component\HttpFoundation\Session\Storage\MockFileSessionStorage">
             <argument>%kernel.cache_dir%/sessions</argument>

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Controller/InjectedFlashbagSessionController.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller;
+
+use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface;
+use Symfony\Component\Routing\RouterInterface;
+
+class InjectedFlashbagSessionController
+{
+    /**
+     * @var FlashBagInterface
+     */
+    private $flashBag;
+
+    /**
+     * @var RouterInterface
+     */
+    private $router;
+
+    public function __construct(
+        FlashBagInterface $flashBag,
+        RouterInterface $router
+    ) {
+        $this->flashBag = $flashBag;
+        $this->router = $router;
+    }
+
+    public function setFlashAction(Request $request, $message)
+    {
+        $this->flashBag->add('notice', $message);
+
+        return new RedirectResponse($this->router->generate('session_showflash'));
+    }
+}

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml

@@ -18,6 +18,10 @@ session_setflash:
     path:     /session_setflash/{message}
     defaults: { _controller: TestBundle:Session:setFlash}
 
+injected_flashbag_session_setflash:
+    path:     injected_flashbag/session_setflash/{message}
+    defaults: { _controller: TestBundle:InjectedFlashbagSession:setFlash}
+
 session_showflash:
     path:     /session_showflash
     defaults: { _controller: TestBundle:Session:showFlash}

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/SessionTest.php

@@ -69,6 +69,29 @@ public function testFlash($config, $insulate)
         $this->assertStringContainsString('No flash was set.', $crawler->text());
     }
 
+    /**
+     * Tests flash messages work when flashbag service is injected to the constructor.
+     *
+     * @dataProvider getConfigs
+     */
+    public function testFlashOnInjectedFlashbag($config, $insulate)
+    {
+        $client = $this->createClient(['test_case' => 'Session', 'root_config' => $config]);
+        if ($insulate) {
+            $client->insulate();
+        }
+
+        // set flash
+        $client->request('GET', '/injected_flashbag/session_setflash/Hello%20world.');
+
+        // check flash displays on redirect
+        $this->assertStringContainsString('Hello world.', $client->followRedirect()->text());
+
+        // check flash is gone
+        $crawler = $client->request('GET', '/session_showflash');
+        $this->assertStringContainsString('No flash was set.', $crawler->text());
+    }
+
     /**
      * See if two separate insulated clients can run without
      * polluting each other's session data.

src/Symfony/Bundle/FrameworkBundle/Tests/Functional/app/Session/config.yml

@@ -5,3 +5,7 @@ services:
     Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\SubRequestController:
         tags:
             - { name: controller.service_arguments, action: indexAction, argument: handler, id: fragment.handler }
+
+    Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\TestBundle\Controller\InjectedFlashbagSessionController:
+        autowire: true
+        tags: ['controller.service_arguments']