From 3c52b289c253a8c5395908d0d3493a879f869d53 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jaime=20Pe=CC=81rez=20Crespo?= <jaime.perez@uninett.no>
Date: Wed, 4 Sep 2019 16:06:57 +0200
Subject: [PATCH] Make sure expired data is actually purged

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
---
 lib/SimpleSAML/Session.php | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/SimpleSAML/Session.php b/lib/SimpleSAML/Session.php
index fd847cb03..5c7a13bcf 100644
--- a/lib/SimpleSAML/Session.php
+++ b/lib/SimpleSAML/Session.php
@@ -453,6 +453,9 @@ class Session implements \Serializable, Utils\ClearableState
      */
     public function save()
     {
+        // clean out old data
+        $this->expireData();
+
         if (!$this->dirty) {
             // session hasn't changed, don't bother saving it
             return;
@@ -894,9 +897,6 @@ class Session implements \Serializable, Utils\ClearableState
         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);
@@ -952,6 +952,7 @@ class Session implements \Serializable, Utils\ClearableState
 
                 if ($ct > $info['expires']) {
                     unset($typedData[$id]);
+                    $this->markDirty();
                 }
             }
         }
@@ -977,8 +978,6 @@ class Session implements \Serializable, Utils\ClearableState
             return null;
         }
 
-        $this->expireData();
-
         if (!array_key_exists($type, $this->dataStore)) {
             return null;
         }
-- 
GitLab