From 5e66f1b21b8741f9a4d54c6a31c7372e44780551 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 ecf169cb0bc88..8ee729015b5fa 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -183,10 +183,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); @@ -501,7 +512,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 8f033ac342d8343e931a2104f0e2ba8f21156469 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 8ee729015b5fa..2c1907cbb3557 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -195,7 +195,6 @@ public function start() if (isset($_SESSION['destroyed'])) { if ($_SESSION['destroyed'] < time() - 300) { $this->destroy(['clear_storage' => true]); - } } $this->validator->validate($this); From 6ee737bb43adcd9b40c170170c5ea8e5d5ad6731 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 2c1907cbb3557..5822566f3903c 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -514,18 +514,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 d63c2c3899e9f91d6afd0a9438a0ecf1de7842e9 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 5822566f3903c..f0547dcdb3ced 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; @@ -512,7 +515,7 @@ public function regenerateId() } if ($this->isSessionExists()) { - //regenerate the session + // Regenerate the session session_regenerate_id(); $newSessionId = session_id(); @@ -523,8 +526,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 5dbce2a48e875cb38d82ba6648b5c04578bf9852 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 f0547dcdb3ced..78524eef8683d 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() * @@ -196,7 +198,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]); } } @@ -514,25 +516,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']); @@ -540,6 +538,7 @@ public function regenerateId() } else { session_start(); } + // @codingStandardsIgnoreEnd $this->storage->init(isset($_SESSION) ? $_SESSION : []); if ($this->sessionConfig->getUseCookies()) { From b1865674bff3c03b92fcc3a2c85f4446735054de 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 78524eef8683d..e1cc81322acd1 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() * @@ -197,11 +192,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); @@ -516,29 +512,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 eabccc83c9ca4e4f55afe6451a93dee3f09115b9 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 e1cc81322acd1..99bfd869f9ceb 100644 --- a/lib/internal/Magento/Framework/Session/SessionManager.php +++ b/lib/internal/Magento/Framework/Session/SessionManager.php @@ -513,7 +513,6 @@ public function regenerateId() } if ($this->isSessionExists()) { - // Regenerate the session session_regenerate_id(); $newSessionId = session_id();