From eee587bc647c053d9bd4eb18c699b00a19535b61 Mon Sep 17 00:00:00 2001 From: Elio Ermini Date: Thu, 3 May 2018 19:56:26 +0200 Subject: [PATCH 1/7] Fix unstable session manager --- .../Framework/Session/SessionManager.php | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/Session/SessionManager.php b/lib/internal/Magento/Framework/Session/SessionManager.php index 662173ad4a09a..23cb853124167 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -180,10 +180,21 @@ public function start() // Need to apply the config options so they can be ready by session_start $this->initIniOptions(); $this->registerSaveHandler(); + if (isset($_SESSION['new_session_id'])) { + // Not fully expired yet. Could be lost cookie by unstable network. + session_commit(); + session_id($_SESSION['new_session_id']); + } $sid = $this->sidResolver->getSid($this); // potential custom logic for session id (ex. switching between hosts) $this->setSessionId($sid); session_start(); + if (isset($_SESSION['destroyed'])) { + if ($_SESSION['destroyed'] < time() - 300) { + $this->destroy(['clear_storage' => true]); + + } + } $this->validator->validate($this); $this->renewCookie($sid); @@ -498,7 +509,31 @@ public function regenerateId() return $this; } - $this->isSessionExists() ? session_regenerate_id(true) : session_start(); + if ($this->isSessionExists()) { + //regenerate the session + session_regenerate_id(); + $new_session_id = session_id(); + + $_SESSION['new_session_id'] = $new_session_id; + + // Set destroy timestamp + $_SESSION['destroyed'] = time(); + + // Write and close current session; + session_commit(); + $oldSession = $_SESSION; //called after destroy - see destroy! + // Start session with new session ID + session_id($new_session_id); + ini_set('session.use_strict_mode', 0); + session_start(); + ini_set('session.use_strict_mode', 1); + $_SESSION = $oldSession; + // New session does not need them + unset($_SESSION['destroyed']); + unset($_SESSION['new_session_id']); + } else { + session_start(); + } $this->storage->init(isset($_SESSION) ? $_SESSION : []); if ($this->sessionConfig->getUseCookies()) { From 3828ac51d0185419c0463cda40734c19d50f186b Mon Sep 17 00:00:00 2001 From: Elio Ermini Date: Fri, 4 May 2018 08:40:22 +0200 Subject: [PATCH 2/7] removed empty line --- lib/internal/Magento/Framework/Session/SessionManager.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/Magento/Framework/Session/SessionManager.php b/lib/internal/Magento/Framework/Session/SessionManager.php index 23cb853124167..d0cbbbbd2c9fa 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -192,7 +192,6 @@ public function start() if (isset($_SESSION['destroyed'])) { if ($_SESSION['destroyed'] < time() - 300) { $this->destroy(['clear_storage' => true]); - } } $this->validator->validate($this); From ac42b70d9332cb848ca639362d4d4136e9385283 Mon Sep 17 00:00:00 2001 From: Elio Ermini Date: Fri, 4 May 2018 08:47:41 +0200 Subject: [PATCH 3/7] amended variable naming --- .../Magento/Framework/Session/SessionManager.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/internal/Magento/Framework/Session/SessionManager.php b/lib/internal/Magento/Framework/Session/SessionManager.php index d0cbbbbd2c9fa..7f3c7a947331e 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -511,18 +511,20 @@ public function regenerateId() if ($this->isSessionExists()) { //regenerate the session session_regenerate_id(); - $new_session_id = session_id(); + $newSessionId = session_id(); - $_SESSION['new_session_id'] = $new_session_id; + $_SESSION['new_session_id'] = $newSessionId; // Set destroy timestamp $_SESSION['destroyed'] = time(); // Write and close current session; session_commit(); - $oldSession = $_SESSION; //called after destroy - see destroy! + + //called after destroy() + $oldSession = $_SESSION; // Start session with new session ID - session_id($new_session_id); + session_id($newSessionId); ini_set('session.use_strict_mode', 0); session_start(); ini_set('session.use_strict_mode', 1); From 5b6e78ea23a7ca6d7149ea63788fd2debe98fdd0 Mon Sep 17 00:00:00 2001 From: Elio Ermini Date: Wed, 9 May 2018 20:40:13 +0100 Subject: [PATCH 4/7] Suppress code standard warning --- lib/internal/Magento/Framework/Session/SessionManager.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/Session/SessionManager.php b/lib/internal/Magento/Framework/Session/SessionManager.php index 7f3c7a947331e..52b2a480400fd 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -5,6 +5,9 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ + +// @codingStandardsIgnoreFile + namespace Magento\Framework\Session; use Magento\Framework\Session\Config\ConfigInterface; @@ -509,7 +512,7 @@ public function regenerateId() } if ($this->isSessionExists()) { - //regenerate the session + // Regenerate the session session_regenerate_id(); $newSessionId = session_id(); @@ -520,8 +523,7 @@ public function regenerateId() // Write and close current session; session_commit(); - - //called after destroy() + // Called after destroy() $oldSession = $_SESSION; // Start session with new session ID session_id($newSessionId); From e688b8a5ed1539f48056d850fb2c7c0529ec43d3 Mon Sep 17 00:00:00 2001 From: Elio Ermini Date: Fri, 11 May 2018 00:43:56 +0100 Subject: [PATCH 5/7] Update from review --- .../Framework/Session/SessionManager.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/internal/Magento/Framework/Session/SessionManager.php b/lib/internal/Magento/Framework/Session/SessionManager.php index 52b2a480400fd..ca8d3d92d7609 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -5,9 +5,6 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ - -// @codingStandardsIgnoreFile - namespace Magento\Framework\Session; use Magento\Framework\Session\Config\ConfigInterface; @@ -18,6 +15,11 @@ */ class SessionManager implements SessionManagerInterface { + /** + * Session destroyed threshold in seconds + */ + const SESSION_DESTROYED_THRESHOLD = 300; + /** * Default options when a call destroy() * @@ -193,7 +195,7 @@ public function start() $this->setSessionId($sid); session_start(); if (isset($_SESSION['destroyed'])) { - if ($_SESSION['destroyed'] < time() - 300) { + if ($_SESSION['destroyed'] < time() - self::SESSION_DESTROYED_THRESHOLD) { $this->destroy(['clear_storage' => true]); } } @@ -511,25 +513,21 @@ public function regenerateId() return $this; } + // @codingStandardsIgnoreStart if ($this->isSessionExists()) { // Regenerate the session session_regenerate_id(); $newSessionId = session_id(); - $_SESSION['new_session_id'] = $newSessionId; - // Set destroy timestamp $_SESSION['destroyed'] = time(); - // Write and close current session; session_commit(); // Called after destroy() $oldSession = $_SESSION; // Start session with new session ID session_id($newSessionId); - ini_set('session.use_strict_mode', 0); session_start(); - ini_set('session.use_strict_mode', 1); $_SESSION = $oldSession; // New session does not need them unset($_SESSION['destroyed']); @@ -537,6 +535,7 @@ public function regenerateId() } else { session_start(); } + // @codingStandardsIgnoreEnd $this->storage->init(isset($_SESSION) ? $_SESSION : []); if ($this->sessionConfig->getUseCookies()) { From f53cfd7a8098896e32210b35e1c90ec3f5df1008 Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Wed, 8 Aug 2018 13:06:33 +0300 Subject: [PATCH 6/7] Fixed minor issues --- .../Framework/Session/SessionManager.php | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/lib/internal/Magento/Framework/Session/SessionManager.php b/lib/internal/Magento/Framework/Session/SessionManager.php index ca8d3d92d7609..0365e5bc49d14 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -15,11 +15,6 @@ */ class SessionManager implements SessionManagerInterface { - /** - * Session destroyed threshold in seconds - */ - const SESSION_DESTROYED_THRESHOLD = 300; - /** * Default options when a call destroy() * @@ -194,11 +189,12 @@ public function start() // potential custom logic for session id (ex. switching between hosts) $this->setSessionId($sid); session_start(); - if (isset($_SESSION['destroyed'])) { - if ($_SESSION['destroyed'] < time() - self::SESSION_DESTROYED_THRESHOLD) { - $this->destroy(['clear_storage' => true]); - } + if (isset($_SESSION['destroyed']) + && $_SESSION['destroyed'] < time() - $this->sessionConfig->getCookieLifetime() + ) { + $this->destroy(['clear_storage' => true]); } + $this->validator->validate($this); $this->renewCookie($sid); @@ -513,29 +509,34 @@ public function regenerateId() return $this; } - // @codingStandardsIgnoreStart if ($this->isSessionExists()) { + // Regenerate the session session_regenerate_id(); $newSessionId = session_id(); $_SESSION['new_session_id'] = $newSessionId; + // Set destroy timestamp $_SESSION['destroyed'] = time(); + // Write and close current session; session_commit(); + // Called after destroy() $oldSession = $_SESSION; + // Start session with new session ID session_id($newSessionId); session_start(); $_SESSION = $oldSession; + // New session does not need them unset($_SESSION['destroyed']); unset($_SESSION['new_session_id']); } else { session_start(); } - // @codingStandardsIgnoreEnd + $this->storage->init(isset($_SESSION) ? $_SESSION : []); if ($this->sessionConfig->getUseCookies()) { From bc15e1ab0df077996e09d11257474adcc0e14899 Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Thu, 9 Aug 2018 12:08:32 +0300 Subject: [PATCH 7/7] Fixed static test failure --- lib/internal/Magento/Framework/Session/SessionManager.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/Magento/Framework/Session/SessionManager.php b/lib/internal/Magento/Framework/Session/SessionManager.php index 0365e5bc49d14..b53c83acb48cf 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -510,7 +510,6 @@ public function regenerateId() } if ($this->isSessionExists()) { - // Regenerate the session session_regenerate_id(); $newSessionId = session_id();