Skip to content

Commit

Permalink
Make sure expired data is actually purged
Browse files Browse the repository at this point in the history
The SimpleSAML\Session::expireData() method did not mark the session as dirty when there was expired data on it, so if nothing else changed, the data was never actually purged. It was done like this by design, but in practice, it seems like sessions aren't modified as often, meaning they end up growing a lot with each state array that's stored on them, and expired data is never removed. We now check for expired data in the save() method (which is run every time a session is destroyed, if not manually) and if there is any, we mark the session as dirty, so that it is actually updated in the backend. Most of the time this will be transparent and have no visible performance hit, as it'll be run after the response is sent, during shutdown.

This closes #1053
  • Loading branch information
jaimeperez committed Sep 4, 2019
1 parent 9d5bd99 commit 3c52b28
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions lib/SimpleSAML/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,9 @@ public static function createSession($sessionId)
*/
public function save()
{
// clean out old data
$this->expireData();

if (!$this->dirty) {
// session hasn't changed, don't bother saving it
return;
Expand Down Expand Up @@ -894,9 +897,6 @@ public function setData($type, $id, $data, $timeout = null)
assert(is_string($id));
assert(is_int($timeout) || $timeout === null || $timeout === self::DATA_TIMEOUT_SESSION_END);

// clean out old data
$this->expireData();

if ($timeout === null) {
// use the default timeout
$timeout = self::$config->getInteger('session.datastore.timeout', null);
Expand Down Expand Up @@ -952,6 +952,7 @@ private function expireData()

if ($ct > $info['expires']) {
unset($typedData[$id]);
$this->markDirty();
}
}
}
Expand All @@ -977,8 +978,6 @@ public function getData($type, $id)
return null;
}

$this->expireData();

if (!array_key_exists($type, $this->dataStore)) {
return null;
}
Expand Down

0 comments on commit 3c52b28

Please sign in to comment.