session_start() ordering problem new to 2.1

[weaverryan]

The Symfony 2.1 branch makes the ProfilerListener a subscribe to the kernel.request event. This means that the ProfilerListener is instantiated very early when all the kernel.request listeners are being prepared (in ContainerAwareEventDispatcher::lazyLoad). This means it's instantiated before any listeners on that event, including the listeners that start the session.

The problem is that the ProfilerListener's instantiation ultimately means that the FacebookSessionPersistence class is instantiated, causing the session to be started before it's intended to be started.

This doesn't seem to cause an outward problem when developing (the session is started a little earler), but it does seem to break session storage in the test environment, because the following happens:

1) FacebookSessionPersistence is instantiated before dispatching kernel.request. This starts the session, and session_id() is generated to a real value.

2) TestSessionStorage sees no cookie (since this is the first request) and sets the session_id to an empty string (https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php#L56)

3) The normal SessionListener is called, which calls Session::start(). This would normally generate the session_id() (since it's blank), but since the session is already started, start does nothing and session_id stays blank.

4) When session persistence kicks (https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php#L78), the session_id is blank. The session is saved, but under the "key" of a blank session_id().

5) On the next request when the session is loading, a brand new session_id is generated once again because the session_id is blank. Session storage does a lookup with this id, which of course brings nothing back. This looks like a new session and the cycle repeats.

The easiest solution would be to remove the session start from FacebookSessionPersistence, but I'm sure that has other consequences.

Ideas on the best way to solve this? Can we remove the session start from the above class?

Thanks!

[schmittjoh]

Shouldn't that better be addressed in Symfony2 itself?

[stof]

I don't think the issue is in Symfony. The real issue is that we call $session->start() inside the constructor of our class, meaning that we will break things as soon as we create this instance early. And the end-user could always write some code calling it early even if we change the priority in Symfony (btw, the ProfilerListener was already a listener in 2.0. The only change is the priority)

We should start the session only the first time we need it in our class

[l3l0]

Hello, I am trying to fix this issue but I noticed that:

BaseFacebook call in the constructor getPersistentData method for 'state' parameter, so $session->start() will be call there, because its the first time when is needed. Maybe you have suggestions how I can handle this ?

[ghost]

@stof is right - sessions should be explicitly loaded when they are needed, and specifically in the context of Sf2 applications, the session should only be started after everything has had a chance to boot and load up. Starting a session on object creation is definitely putting the cart before the horse in Sf2 paradigm since FrameworkBundle is configured to start the session when the request cycle starts (which is after everything has booted up).

I guess the problem is \BaseFacebook does work in it's constructor so maybe the solution would be not to call the parent constructor, make a method called initialise, and add a listener after the session start listener which will then calls initialize (which calls the \BaseFacebook::__construct() manually. It would stop the chicken ad egg thing and mean symfony2 regains control over when the session should be started.

[MattKetmo]

Is there any update about this issue?

For more feedbacks, I also get errors when running my tests when using FosFacebookBundle:

Cannot set session ID after the session has started.
500 Internal Server Error - LogicException

But, as @drak pointed it, removing those two lines in FacebookSessionPersistence

    $this->session->start();
    parent::__construct($config);

...solves (temporary?) this problem.

[ghost]

Yes, as far as I am concerned, this is a problem with the facebook library - the solution is to override the constructor and do things the Sf2 way. While running session_start() can be made to work at a stretch it will then break tests - that's why this particular error exception is present in the mock classes. See https://github.com/symfony/symfony-docs/pull/1376/files#L3R38

[bmeynell]

This is causing our test suite to crash as well.

Per @MattKetmo here's a temporary work around for tests: https://gist.github.com/2786531

[hardchor]

Could this also have to do with the session auto-starting with every request even if the firewall allows anonymous users?

[gquemener]

See #183

[vazgen]

Hi, is there any progress?