Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.5] Fix NullSessionDriver due to upstream changes #22314

Merged
merged 1 commit into from
Dec 5, 2017
Merged

[5.5] Fix NullSessionDriver due to upstream changes #22314

merged 1 commit into from
Dec 5, 2017

Conversation

laurencei
Copy link
Contributor

@laurencei laurencei commented Dec 5, 2017

This is an attempt to fix #22302 that has started occuring due to upstream changes in the Symfony session (see the issue for a very good detailed explanation of the problem by @acarpio89).

The reason the changes only seems to affect the NullSessionDriver is because Laravel actually uses its own implementations of every other driver, and was only using the Symfony Null driver.

So this PR just creates our own Null driver, and we no longer rely on Symfony for it.

On a seperate issue - this is not picked up by any framework tests - because the tests all mock the session handler, and never try to actually create a session (from what I can see). I tried to create a failing test to capture this situation, but I cant work out how to do it inside a framework test?

Would be nice to add a test around this if someone can point me in the right direction.

@taylorotwell taylorotwell merged commit e19b54c into laravel:5.5 Dec 5, 2017
@laurencei
Copy link
Contributor Author

@taylorotwell - thanks for the quick merge.

Any idea how I can write a test around this? I cant seem to actually generate a session inside the test suite framework. It would be nice to write an integration test against the array driver or something?

@laurencei laurencei deleted the fixNullSessions branch December 5, 2017 21:28
@danijelk
Copy link
Contributor

danijelk commented Dec 9, 2017

@laurencei Guess this is related here?

auth()->loginUsingId($widget->user_id);

Session name cannot be empty, did you forget to call "parent::open()" in "Symfony\Component\HttpFoundation\Session\Storage\Handler\NullSessionHandler"?.

Can't really find the culprit locally on my mac, this only happens in production after latest update.

  • I have set the | Session Cookie Name in session.php
  • Using redis as driver

Does one need to update to symfony 4.x?

@laurencei
Copy link
Contributor Author

Are you running Laravel 5.5.24 or higher?

It doesnt really make much sense, because you are using the Redis driver. It should never call the Null handler.

@danijelk
Copy link
Contributor

danijelk commented Dec 9, 2017

Indeed, was a symlink error in the project. After 5.5.24 update it works good, seems like only symfony was updated without this patch. Thanks for quick reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants