Skip to content

Commit b40ca8a

Browse files
committed
MAGETWO-60910: Session initialized during reindex from CLI and result fatal error
- Don't start session if cli used.
1 parent d88cf06 commit b40ca8a

File tree

4 files changed

+122
-43
lines changed

4 files changed

+122
-43
lines changed

dev/tests/integration/etc/di/preferences/ce.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,5 @@
2727
\Magento\Framework\App\Config\ScopeConfigInterface::class => \Magento\TestFramework\App\Config::class,
2828
\Magento\Framework\App\ResourceConnection\ConfigInterface::class =>
2929
\Magento\Framework\App\ResourceConnection\Config::class,
30+
\Magento\Framework\Session\SessionStartChecker::class => \Magento\TestFramework\Session\SessionStartChecker::class
3031
];
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\TestFramework\Session;
10+
11+
/**
12+
* Class to check if session can be started or not. Dummy for integration tests.
13+
*/
14+
class SessionStartChecker extends \Magento\Framework\Session\SessionStartChecker
15+
{
16+
/**
17+
* Can session be started or not.
18+
*
19+
* @return bool
20+
*/
21+
public function check() : bool
22+
{
23+
return true;
24+
}
25+
}

lib/internal/Magento/Framework/Session/SessionManager.php

Lines changed: 58 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class SessionManager implements SessionManagerInterface
3636
/**
3737
* Validator
3838
*
39-
* @var \Magento\Framework\Session\ValidatorInterface
39+
* @var ValidatorInterface
4040
*/
4141
protected $validator;
4242

@@ -50,28 +50,28 @@ class SessionManager implements SessionManagerInterface
5050
/**
5151
* SID resolver
5252
*
53-
* @var \Magento\Framework\Session\SidResolverInterface
53+
* @var SidResolverInterface
5454
*/
5555
protected $sidResolver;
5656

5757
/**
5858
* Session config
5959
*
60-
* @var \Magento\Framework\Session\Config\ConfigInterface
60+
* @var Config\ConfigInterface
6161
*/
6262
protected $sessionConfig;
6363

6464
/**
6565
* Save handler
6666
*
67-
* @var \Magento\Framework\Session\SaveHandlerInterface
67+
* @var SaveHandlerInterface
6868
*/
6969
protected $saveHandler;
7070

7171
/**
7272
* Storage
7373
*
74-
* @var \Magento\Framework\Session\StorageInterface
74+
* @var StorageInterface
7575
*/
7676
protected $storage;
7777

@@ -92,6 +92,11 @@ class SessionManager implements SessionManagerInterface
9292
*/
9393
private $appState;
9494

95+
/**
96+
* @var SessionStartChecker
97+
*/
98+
private $sessionStartChecker;
99+
95100
/**
96101
* @param \Magento\Framework\App\Request\Http $request
97102
* @param SidResolverInterface $sidResolver
@@ -102,7 +107,10 @@ class SessionManager implements SessionManagerInterface
102107
* @param \Magento\Framework\Stdlib\CookieManagerInterface $cookieManager
103108
* @param \Magento\Framework\Stdlib\Cookie\CookieMetadataFactory $cookieMetadataFactory
104109
* @param \Magento\Framework\App\State $appState
110+
* @param SessionStartChecker|null $sessionStartChecker
105111
* @throws \Magento\Framework\Exception\SessionException
112+
*
113+
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
106114
*/
107115
public function __construct(
108116
\Magento\Framework\App\Request\Http $request,
@@ -113,7 +121,8 @@ public function __construct(
113121
StorageInterface $storage,
114122
\Magento\Framework\Stdlib\CookieManagerInterface $cookieManager,
115123
\Magento\Framework\Stdlib\Cookie\CookieMetadataFactory $cookieMetadataFactory,
116-
\Magento\Framework\App\State $appState
124+
\Magento\Framework\App\State $appState,
125+
SessionStartChecker $sessionStartChecker = null
117126
) {
118127
$this->request = $request;
119128
$this->sidResolver = $sidResolver;
@@ -124,11 +133,15 @@ public function __construct(
124133
$this->cookieManager = $cookieManager;
125134
$this->cookieMetadataFactory = $cookieMetadataFactory;
126135
$this->appState = $appState;
136+
$this->sessionStartChecker = $sessionStartChecker ?: \Magento\Framework\App\ObjectManager::getInstance()->get(
137+
SessionStartChecker::class
138+
);
127139
$this->start();
128140
}
129141

130142
/**
131-
* This method needs to support sessions with APC enabled
143+
* This method needs to support sessions with APC enabled.
144+
*
132145
* @return void
133146
*/
134147
public function writeClose()
@@ -163,47 +176,49 @@ public function __call($method, $args)
163176
*/
164177
public function start()
165178
{
166-
if (!$this->isSessionExists()) {
167-
\Magento\Framework\Profiler::start('session_start');
168-
169-
try {
170-
$this->appState->getAreaCode();
171-
} catch (\Magento\Framework\Exception\LocalizedException $e) {
172-
throw new \Magento\Framework\Exception\SessionException(
173-
new \Magento\Framework\Phrase(
174-
'Area code not set: Area code must be set before starting a session.'
175-
),
176-
$e
177-
);
178-
}
179+
if ($this->sessionStartChecker->check()) {
180+
if (!$this->isSessionExists()) {
181+
\Magento\Framework\Profiler::start('session_start');
182+
183+
try {
184+
$this->appState->getAreaCode();
185+
} catch (\Magento\Framework\Exception\LocalizedException $e) {
186+
throw new \Magento\Framework\Exception\SessionException(
187+
new \Magento\Framework\Phrase(
188+
'Area code not set: Area code must be set before starting a session.'
189+
),
190+
$e
191+
);
192+
}
179193

180-
// Need to apply the config options so they can be ready by session_start
181-
$this->initIniOptions();
182-
$this->registerSaveHandler();
183-
if (isset($_SESSION['new_session_id'])) {
184-
// Not fully expired yet. Could be lost cookie by unstable network.
185-
session_commit();
186-
session_id($_SESSION['new_session_id']);
187-
}
188-
$sid = $this->sidResolver->getSid($this);
189-
// potential custom logic for session id (ex. switching between hosts)
190-
$this->setSessionId($sid);
191-
session_start();
192-
if (isset($_SESSION['destroyed'])
193-
&& $_SESSION['destroyed'] < time() - $this->sessionConfig->getCookieLifetime()
194-
) {
195-
$this->destroy(['clear_storage' => true]);
196-
}
194+
// Need to apply the config options so they can be ready by session_start
195+
$this->initIniOptions();
196+
$this->registerSaveHandler();
197+
if (isset($_SESSION['new_session_id'])) {
198+
// Not fully expired yet. Could be lost cookie by unstable network.
199+
session_commit();
200+
session_id($_SESSION['new_session_id']);
201+
}
202+
$sid = $this->sidResolver->getSid($this);
203+
// potential custom logic for session id (ex. switching between hosts)
204+
$this->setSessionId($sid);
205+
session_start();
206+
if (isset($_SESSION['destroyed'])
207+
&& $_SESSION['destroyed'] < time() - $this->sessionConfig->getCookieLifetime()
208+
) {
209+
$this->destroy(['clear_storage' => true]);
210+
}
197211

198-
$this->validator->validate($this);
199-
$this->renewCookie($sid);
212+
$this->validator->validate($this);
213+
$this->renewCookie($sid);
200214

201-
register_shutdown_function([$this, 'writeClose']);
215+
register_shutdown_function([$this, 'writeClose']);
202216

203-
$this->_addHost();
204-
\Magento\Framework\Profiler::stop('session_start');
217+
$this->_addHost();
218+
\Magento\Framework\Profiler::stop('session_start');
219+
}
220+
$this->storage->init(isset($_SESSION) ? $_SESSION : []);
205221
}
206-
$this->storage->init(isset($_SESSION) ? $_SESSION : []);
207222
return $this;
208223
}
209224

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
declare(strict_types=1);
8+
9+
namespace Magento\Framework\Session;
10+
11+
/**
12+
* Class to check if session can be started or not.
13+
*/
14+
class SessionStartChecker
15+
{
16+
/**
17+
* @var bool
18+
*/
19+
private $checkSapi;
20+
21+
/**
22+
* @param bool $checkSapi
23+
*/
24+
public function __construct(bool $checkSapi = true)
25+
{
26+
$this->checkSapi = $checkSapi;
27+
}
28+
29+
/**
30+
* Can session be started or not.
31+
*
32+
* @return bool
33+
*/
34+
public function check() : bool
35+
{
36+
return !($this->checkSapi && PHP_SAPI === 'cli');
37+
}
38+
}

0 commit comments

Comments
 (0)