Symfony: [Session][Test] Cannot set session ID after the session has started.

[havvg]

Hi there,

the exception is thrown by this line:
https://github.com/symfony/symfony/blob/2.5/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php#L134

My setup works completely fine within a valid PHP environment (without TestSessionListener) - data getting stored in the session before the TestSessionListener::onKernelRequest has been executed (using MockFileSessionStorage).

What's the intention of this exception? I can't get my head around it. The TestSessionListener is explicitly doing this (setId) and if there is another object accessing the session before the TestSessionListener sets the id, an id is automatically generated. The listener is not correctly mimicking the session creation of PHP and this exception makes it (for some parts) unusable.

If there is no actual use on this exception, it may be removed, no?

[jakzal]

Indeed it doesn't seem to make any difference in case of MockArraySessionStorage, as the session data is stored in memory. However, in case of MockFileSessionStorage, which extends the MockArraySessionStorage it might make a difference.

If a session is started before the previous id is set from a cookie, you'd get a new session on each request, no?

[havvg]

Indeed, that's another upcoming issue. The data is not migrated and in addition any data accessed from the "new" session is therefore not from the expected source. Even if the listener would migrate, any object that has accessed the session before, would have based its behaviour on the wrong dataset.

I'm getting more and more convinced, that the test session needs a different approach on bootstrapping. The current listener seems not valid, it's far too late.

[jakzal]

@havvg would your issue be fixed if TestSessionListener was triggered earlier?

[havvg]

Not in sense of "higher listener priority", no.

I tried something different, which would work, but it's not a valid solution as it (injecting the id in Kernel::boot, just for the sake of trying it). I'm thinking about a proper solution to achieve this, maybe a factorized service would do the trick, that way the container (and thus the session itself) has immediate access to the correct data. Tricky one, this issue is :)

[jakzal]

In that case, would you mind forking the standard edition, to reproduce this issue?

[havvg]

Yes, I will create one.

[stof]

injecting the id during the boot is wrong. A booted kernel could technically handle several requests, and the session id is tied to a request

[havvg]

Yes, I know, it was just a proof of concept and easy to "hack in".

[jakzal]

@havvg any news?

[havvg]

@jakzal Sorry for the delay, I just set up a simplified version of how to produce the issue.

[spezifanta]

What's the status? Ran into this today.

[sbergfeld]

Me too. Any ideas?

[adamelso]

@jakzal I'm having the same issue also when running Behat 3. I started getting this exception when I upgraded Symfony from 2.6.4 to 2.6.5.

[ctrl-f5]

I'm also running into this, reverting to 2.6.4 did not help in my case...

[adamelso]

I've discovered that I had a listener subscribed to the kernel.request event that used the Sylius Money Helper service. This had since been modified to be injected with a LocaleContext, a service that depends on the session: https://github.com/Sylius/Sylius/pull/2579

However my listener had a priority of -64, so it should have been triggered after the TestSessionListener, right? TestSessionListener has a priority of 192.

[jakzal]

@adamelso would you be able to fork the Symfony SE and isolate this issue on a new branch?

[adamelso]

@jakzal sure, any particular branch I should fork from ?

[jakzal]

The oldest supported branch that you can reproduce this issue on :)

[javiereguiluz]

I'm also suffering this issue in my tests. Any news on the potential fix? Thanks.

[adamelso]

Sadly I haven't had much luck reproducing the issue in either 2.6 or 2.7.

The issue I was having was because of a listener that set an object (similar to the GlobalVariables one from Symfony Templating) as a Twig Global and this object also included Sylius Money Helper which now depends on the session. I fixed the problem it by registering a Twig Extension instead, which was probably the correct way to do it anyway.

I'll still try to reproduce it if I can.

[jakzal]

@javiereguiluz perhaps you could provide a SE fork with this issue reproduced?

[sbergfeld]

@adamelso Thank you. Finally I found it:

I defined a twig globals var in config.yml and I assigned a service. This service had @session as an argument.This caused all the trouble.

[maryo]

I've also encountered this issue in functional test extending WebTestCase.
It was working fine before i disabled kernel rebooting (by calling disableReboot on test.client.class because of DB isolation - i want to share the same connection so it can run in one transaction).

There is a TestSessionListener class which sets session ID if present in cookies on every master request.
Problem is when the session is shared between requests (which is also caused by disabling the kernel reboot). Then it tries to set the session id on already started session in MockArraySessionStorage.php:142

    public function setId($id)
    {
        if ($this->started) {
            throw new \LogicException('Cannot set session ID after the session has started.');
        }

        $this->id = $id;
    }

In this case this call is redundant since the same session ID is already present in the session.

I temporarily fixed it by extending MockArraySessionStorage (or MockFileSessionStorage) like this.

    /**
     * {@inheritdoc}
     */
    public function setId($id)
    {
        if ($this->id !== $id) {
            parent::setId($id);
        }
    }

It is simpler than extending the TestSessionListener class. But if I understand it correctly then the listener is the place where the condition should be present.

Should it be fixed? Should i provide a PR? Have someone already provided a failing test case?

[toby-griffiths]

I'm having this problem with a Behat test that generates a CSRF token during the test steps, as shown below…

    /**
     * @When /^I go to the application admin archive page for "(?P<status>[^"]*)" application (?P<number>\d+) with a valid token$/
     *
     * @param string $status
     * @param int    $number
     */
    public function iGoToTheApplicationAdminArchivePageForApplicationWithAValidToken($status, $number)
    {
        $tokenManager = $this->kernel->getContainer()->get('security.csrf.token_manager');
        $token        = $tokenManager->getToken(ApplicationAdminController::CSRF_ARCHIVE);

        $this->visitAdminPage('archive', $status, $number, ['token' => $token]);
    }




    /**
     * @param string $page
     * @param string $status
     * @param int    $number
     * @param array  $routeArgs
     */
    private function visitAdminPage($page, $status = null, $number = null, array $routeArgs = [])
    {
        // Gets $route
        // ...

        $this->minkContext->visitPath($route);
    }

Anything else I can do to help this along?

[zyphlar]

I fixed this during a 2.5 -> 2.6 upgrade by finding all references to session in my source and removing them one-by-one til I found the culprit (as usual, the very last one.) It seems that passing the session to a service is OK, but accessing its variables or modifying it in the service's constructor is a no-no unless the session is started. You can check this by checking if($session->isStarted()).

[adsc]

@maryo Thank you for this fix, we also ran into this problem when we switched to sqlite in-memory db for functional tests, and thus had to call disableReboot() on the client.

[mcfedr]

Looking at this same issue, I am wondering if it makes more sense to put the fix in TestSessionListener

[yvoloshin]

@maryo Could you please add more details on your fix? Specifically, where did you add the new function and how did you call it? I'm new to testing in Symfony and having trouble figuring it out.Thank you!

[mcfedr]

If you interested, I have worked around this by clearing the session between requests.
In one test:

$client->disableReboot();
$client->request...
...
$client->getCookieJar()->clear();
$client->request...

This means I didn't have to apply fixes inside the Symfony code.

[mcfedr]

@yvoloshin The fixes proposed by @maryo are in the class Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage

[hugomn]

@sbergfeld I have the same issue here. After I've created a twig extension that had @session as an argument, I had the issue. How did you solve that?

[yvoloshin]

One of my coworkers solved this problem. The config_test.yml had these lines:

framework:
    test: ~
    profiler:
        collect: false

Deleting test: ~ from config_test.yml got rid of the error.

[pharman]

Thanks @yvoloshin, I am literally working on this same problem at this exact time. Now fixed thanks to you!

[hugomn]

In my case I was using the @session service in the Twig Extension constructor, like:

public function __construct($session) {
  $this->timezone = $this->session->get('timezone', 'UTC');
}

The solution was just to store @session and use it later:

public function __construct($session) {
  $this->session = $session;
}

Thanks, anyway.

[jkobus]

Im getting this on symfony 3.0.3 with LiipFunctionalTestBundle. Removing "test" entry as @yvoloshin suggested also removed the test client, so it does not solve the problem.

[oscarjg]

Anyone have a final solution about this? I tried all but the problem has not fixed. I create my custom file session storage as commented and works, but when I testing my controllers as a functional test the CSRF protection of my form change after submit and I'm thinking this issue cause the problem. Remove test entry it's not an option as @jkobus says and I check all my services who has sessions dependencies and any service access to my session properties in the construct.

Thanks!

[mcfedr]

Consider disabling csrf in config_test.yml

[oscarjg]

@mcfedr Thanks. I did it and with combinations with my "custom" session provider seems it works. Do you know if someone is working to fix this issue or maybe there are not solution and this is the best way by the moment?

[rgarcia-martin]

Hi everybody!

This work for me: Im doing functionality tests in my Symfony 2.6 app. That test go against routes protected and i need to do the requests with an UsernamePasswordToken setted at the session.

-The cookie always have the same id that the session, but that Exception is thrown when the cookie is setted and TestSessionListener check that the cookie have an item with the session name.

My solution:

• Create an class than extends Symfony\Bundle\FrameworkBundle\EventListenerTestSessionListener
• Set "onKernelRequest" with the code:

public function onKernelRequest(GetResponseEvent $event)
    {
        if (!$event->isMasterRequest()) {
            return;
        }

        // bootstrap the session
        $session = $this->getSession();
        if (!$session) {
            return;
        }

        $cookies = $event->getRequest()->cookies;

        if ($cookies->has($session->getName())) {
            if($session->getId() != $cookies->get($session->getName())){
                $session->setId($cookies->get($session->getName()));
            }
        }
    }

• The cookie[sessionName] value and session[id] are the same...
• Set in config_test.yml :

parameters:
    test.session.listener.class: "Path\To\Your\Class"

Why it have to set the id again and expose us to get an exception if the session is started?

I hope help you! Bye!

[sarven]

I had randomly the same problem in Symfony 3.2.* during functional tests. Solution presented by @rgarcia-martin didn't work for me, so I made own with following code:

<?php

namespace CoreBundle\Event;

use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\EventListener\TestSessionListener as BaseTestSessionListener;

class TestSessionListener extends BaseTestSessionListener
{
    /**
     * @var ContainerInterface
     */
    protected $container;

    /**
     * TestRequestListener constructor.
     * @param ContainerInterface $container
     */
    public function __construct(ContainerInterface $container)
    {
        $this->container = $container;
    }

    /**
     * @return null|SessionInterface
     */
    protected function getSession() : ?SessionInterface
    {
        if (!$this->container->has('session')) {
            return null;
        }

        return $this->container->get('session');
    }

    /**
     * @param GetResponseEvent $event
     */
    public function onKernelRequest(GetResponseEvent $event)
    {
        if (!$event->isMasterRequest()) {
            return;
        }

        // bootstrap the session
        $session = $this->getSession();
        if (!$session) {
            return;
        }

        $cookies = $event->getRequest()->cookies;

        if ($cookies->has($session->getName())) {
            if (!$session->isStarted()) {
                $session->setId($cookies->get($session->getName()));
            }
        }
    }
}

and in config_test.yml :

services:
    test.session.listener:
        class: AppBundle\Event\TestSessionListener
        arguments:
            - '@service_container'
        tags:
            - { name: kernel.event_subscriber }

Seems it works.

[JoshuaEstes]

Hoping this will help someone. I'm using the Liip Functional Test Bundle and was getting this issue. The way I solved it was to save the session before a request was made.

Example:

$client = $this->makeClient();
$client->getContainer()->get('session')->set('session_key_to_use', 'value_for_key');
$client->getContainer()->get('session')->save(); // This seems to make it work
$crawler = $client->request('GET', '/path/to');

---

config_test.yml

framework:
  test: ~
  session:
    name: MOCKSESSID
    storage_id: session.storage.mock_file

Symfony Version: 3.4.2
Liip Functional Test Version: 1.9.0

Hope this helps

[soullivaneuh]

Got the same issue with Liip Functional Test Bundle too, and Symfony 4.0.

Workaround for my case, inspired from https://github.com/symfony/symfony/issues/13450#issuecomment-147447252:

use Symfony\Component\HttpFoundation\Session\Storage\MockFileSessionStorage as BaseSessionStorage;

final class MockFileSessionStorage extends BaseSessionStorage
{
    public function setId($id)
    {
        if ($this->id !== $id) {
            parent::setId($id);
        }
    }
}

Then on the Kernel class:

use App\Tests\TestCase\MockFileSessionStorage;
use Symfony\Component\HttpKernel\Kernel as BaseKernel;

class Kernel extends BaseKernel implements CompilerPassInterface
{
    public function process(ContainerBuilder $container): void
    {
        if ('test' === $this->environment) {
            $container->getDefinition('session.storage.mock_file')->setClass(MockFileSessionStorage::class);
        }
    }

Work for me. But I'm sure the exception is not here for nothing. :-)

What is the state of this issue?

[nicolas-grekas]

PR pending if a solution has been identified...

[soullivaneuh]

@nicolas-grekas Well, the issue is we found some workaround, but it seems we don't really know why we have this "big". :confused:

[rpkamp]

I ran into the same problem and found that the issue is that I have an event subscriber with a higher priority than Symfony's TestSessionListener that interacts with the session.

So what happens is first my listener (priority 2048) calls has on the session, but the session hasn't started yet, so it is started with a random ID generated by MockArraySessionStorage.

After that the TestSessionListener (priority 192) is called and wants to set the ID of the session it retrieved from a session cookie, but this fails because a session was already started by my call to has in my own listener, and you get the exception Cannot set session ID after the session has started.

I've "solved" this for now with a CompilerPass that removes the default tags from TestSessionListener and replaces it with tags with a higher priority for kernel.request than my own listener, so the TestSessionListener is called before my own listener, and that solves the problem for me.

$container->getDefinition('test.session.listener')->clearTags();
$container->getDefinition('test.session.listener')->addTag(
    'kernel.event_listener',
    [
        'event' => 'kernel.request',
        'method' => 'onKernelRequest',
        'priority' => 2049 // priority of my listener + 1
    ]
);
$container->getDefinition('test.session.listener')->addTag(
    'kernel.event_listener',
    [
        'event' => 'kernel.response',
        'method' => 'onKernelResponse',
        'priority' => -1000 // same as original, doesn't really matter
    ]
);

Without resorting to gigantic numbers for the priority of TestSessionListener I'm not really sure how to structurally solve this problem though. And even with a very high priority there is always someone that needs an even higher priority for one reason or another and breaks it again.

Maybe there is a way to retrieve all events subscribed to kernel.request, finds the highest priority, and register TestSessionListener with that highest priority + 1 in a CompilerPass somewhere? Rather nasty but would work.

Or maybe we could pass the RequestStack to the MockFileSessionStorage and let it use the cookie in the master request (if it exists) instead of generating it's own ID. That would couple the two, but they are already coupled at the moment through the TestSessionListener.
That would require the RequestStack have a master request upon first call though, and I'm not sure that something we can and/or should guarantee.

[aurelijusrozenas]

Disclaimer: just trying stuff blindly, I don't really understand why this works. I do not fully undestand the problem.
Functional tests started failing after update to:
Liip Functional Test Bundle: 1.9.3
Symfony 3.4.11
Solution: same as @sarven suggested, with addition of $session->save(); before $cookies = $event->getRequest()->cookies;. Sarven's original solution is not working for me. Other solutions are also not working.

[tgalopin]

I stumbled upon this problem and may have a fix with https://github.com/symfony/symfony/pull/28433. Don't hesitate to try and give your opinion on the PR or here. Thanks!

[tom10271]

I had randomly the same problem in Symfony 3.2.* during functional tests. Solution presented by @rgarcia-martin didn't work for me, so I made own with following code:

<?php

namespace CoreBundle\Event;

use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\EventListener\TestSessionListener as BaseTestSessionListener;

class TestSessionListener extends BaseTestSessionListener
{
    /**
     * @var ContainerInterface
     */
    protected $container;

    /**
     * TestRequestListener constructor.
     * @param ContainerInterface $container
     */
    public function __construct(ContainerInterface $container)
    {
        $this->container = $container;
    }

    /**
     * @return null|SessionInterface
     */
    protected function getSession() : ?SessionInterface
    {
        if (!$this->container->has('session')) {
            return null;
        }

        return $this->container->get('session');
    }

    /**
     * @param GetResponseEvent $event
     */
    public function onKernelRequest(GetResponseEvent $event)
    {
        if (!$event->isMasterRequest()) {
            return;
        }

        // bootstrap the session
        $session = $this->getSession();
        if (!$session) {
            return;
        }

        $cookies = $event->getRequest()->cookies;

        if ($cookies->has($session->getName())) {
            if (!$session->isStarted()) {
                $session->setId($cookies->get($session->getName()));
            }
        }
    }
}

and in config_test.yml :

services:
    test.session.listener:
        class: AppBundle\Event\TestSessionListener
        arguments:
            - '@service_container'
        tags:
            - { name: kernel.event_subscriber }

Seems it works.

Hi @sarven and other,

This solution works really fine for BrowserKit but I cannot login with Selenium. Any thoughts or ideas why this is happening?

Thank you for replying

[aurelijusrozenas]

@tom10271 did you clear cache for test env? I have to clear cache everytime I run BrowserKit tests and then run Selenium and vise versa.