Closed
Description
So are dozens of Session bug reports across the Symfony and Silex and other related libraries. Most of them are because of wrong usage or misunderstanding. This ticket sums up how I see sessions should behave after researching many, many tickets.
Session State
/**
* Whether the session can be resumed from the previous request.
*
* It means that there is a cookie with the given session name.
*
* @return bool
*/
public function isResumable();
/**
* Returns whether the session was started.
*
* When the session is saved, i.e. not active anymore, it will still return true. It only means that start() was called at least once (directly or indirectly).
*
* @return bool
*/
public function isStarted();
/**
* Returns whether the session is currently open, i.e. started but not saved yet.
*
* Equivalent to session_status() === PHP_SESSION_ACTIVE. But needs special treatment for PHP 5.3.
*
* @return bool
*/
public function isActive();
Session Usage
Session methods should throw exceptions when misused and when this misuse would cause wrong expectations. If the "misuse" does not cause side-effects or unexpected behavior, it should not throw exceptions.
SessionStorage::regenerateshould throw exception if no session is active because otherwise it has no effect. This fixes Session invalidate does not work (session not started automatically) silexphp/Silex#746 and https://bugs.php.net/bug.php?id=49462SessionStorage::startshould NOT throw an exception because starting a session twice makes no sense but has no downsides.- If the session is already started by symfony, just ignore the start (this is already how it is).
- If the session was started by third party (e.g. facebook sdk) or legacy apps and is still ACTIVE, save and close the third party session and start the symfony session instead so it uses it's configuration (save handler especially). Optionally, one can also merge the current
$_SESSIONdata into the symfony session data, in case you need access to the third party session data from within the symfony session. SessionStorage::saveshould throw an exception if session is not active because there is something wrong in program logic. You cannot save something that does not exist.SessionStorage::setIdshould throw an exception if session is active (partly as-is). But the question is why there is a setter at all? There should not be a case where you need to explicitly set an id. It's exactly the point that the id is random and not guessable.
Session Lifecycle
Always save session before sending response / doing a redirect. This will fix many common problems:
- When doing a redirect after a
Session::regeneratelogin, the session data is actually available on the next request. Fixes NativeSessionStorage::regenerate bug #7380 and Symfony 2.2.1 - session lost after success login rediraction #7885 and https://bugs.php.net/bug.php?id=61470. Locking won't help in this case because the ID is different, which is the whole point. [HttpFoundation] fixed issue with session_regenerate_id (closes #7380) #8270 should be reverted. - Locking solves all other cases with concurrency problems. See [HttpFoundation] Strange behaviour with empty session, Swift and fastcgi_finish_request function #6417 But making sure that the session is saved before sending response will improve performance when people forgot to call save themselves. It prevents that the session is locked longer than needed due to long running things after sending the response.
- And if the dev uses a save handler without locking (not available or on purpose, see [HttpFoundation] Session handlers should use a lock #4976), then it also ensures the data is available on the next request how it is expected.
Auto-start behavior when reading/writing
See also #6036
- Reading from the session when there is no session cookie, i.e. there is no previous session: It SHOULD NOT start the session. There is no reason to create a session with a cookie when only reading data. It just means the data is not available as we already know since there is no session. This is also what @TerjeBr suggested. @Drak argument that people need to check themselves whether the session is active is also possible. But the example he gave
{% if app.session.isstarted() %}{% for flashMessage in app.session.flashbag.get('notice') %}is wrong. If the session is closed/saved this won't even work and not show the messages. So actually one would need to checkRequest::hasPreviousSessioneach time one reads session data at the moment. But the session part does not even have a method for that. So when injecting the session alone (without request) you don't have access to this important method. So in reality, forcing users to check the session state everywhere where they read data is not viable. - Reading from the session when there is IS a session cookie . It should auto-start the session which reads the information stored in the session and makes them available. Otherwise reading data would not find the data unless it has been started manually which is also not developer friendly.
- Reading from the session when the session has already been started in the current request but is currently closed/saved: In this case we don't necessarily need to re-start the session because the data is already available. This is a performance enhancement because it prevents to read the data again that we already have. The only disavantage is that it does not synchronize the data written in another request between your
save()and theget(). But this is highly unlikely and rather strange case anway. And devs could just callstart()themselves if they want that sync. - Writing to the session should always start the session so that the data is actually persisted. Otherwise there would be no point in writing to the session when it's not available in the next request. Zend Session raises an immutable exception when writing to a session that is already closed/saved. But I don't think that's a good idea because then you cannot add session data later in the lifecycle anymore.