Hey!

I think@mtarld has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Hi! I think the following lines would have to be deleted. It just does not make sense to remove the cookie if the session bags are empty, because that does not mean the session is empty (and used by other application):https://github.com/symfony/symfony/blob/v5.4.0/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php#L154-L162

@devnix this lines can not be removed as they handles the correct handling of a logout when running swoole and roadrunner. I'm currently thinking about check if the PHP bridge is activated and then go with the old behaviour and let PHP handle the session cookie then again. Means that swoole and roadrunner could not be used with that applications correctly but I think that this application don't even use the symfony/runtime so I think that would be fine.

@alexander-schranz alright, that's what I feared (I didn't make directly a PR because I suspected I was going to miss a fact like that).

Speaking from ignorance: maybe the listener could call a method defined bySessionStorageInterface, so each session storage would know how they have to handle by implementing that method?

@devnix theisEmpty is currently a way to know ifinvalidate was called:

@devnix found that I can check if the session was even changed using the session usage index. Can you check this change with your application?

I will be able to check it next monday!

I have a wild guess: if a session is started by another application, and later a Symfony controller is called without touching the session, will not be deleted because the usage index would be zero?

This can even happen with coexisting, separated web applications, not only with legacy applications integrating Symfony

@devnix Thank you for the feedback. Make sense. I tried to create test cases for it and check now also for$_SESSION to avoid these problems with other native sessions variables written before.

/cc@jderusse

I will try the patch ASAP, as I don't have access to the repository right now. Thank you so much,@alexander-schranz

I know, I'm late to the conversation but I think it would be a good idea to give the person, implementing the session handling a way to control the behavior of creating/deleting the session cookie (as@devnix suggested).

In my case we have a centralized login which creates a session (in Redis) and writes the cookie.
Our applications (around 20) are using this already created session.

Now we want to migrate those applications away from the current framework to Symfony, so I implemented a custom authenticator for this session but also wanted to reuse this session in a custom SessionStorage.

This worked up until Symfony 5.4 which now kills the externally generated session cookie.
So, a way for my SessionStorage to tell Symfony: "don't mess with the session cookie" would be nice.

My solution now is to have two sessions. One created by our login application and one created by Symfony, using one Redis connection (singleton via DI).
This also doesn't work as intended because of#44616 but is fixed with#44657

@johannes85 as discussed in#44634 (comment) there would be better options. But are not possible in a bugfix as a bugfix is not allowed to extend the public API.

Thank you@alexander-schranz.

Thx all for helping, testing and providing feedback!

@alexander-schranz could you please have a look at branch 6.0?
I feel like my merge is wrong there 😬 (tests of HttpKernel are failing).

@nicolas-grekas see#45192