Skip to content

Commit 1de2870

Browse files
sidolovsidolov
authored
ENGCOM-5688: Throw exception to prevent infinite loop caused by third party code. #18678 
2 parents c2bd3ae + 173b65b commit 1de2870

File tree

7 files changed

+265
-0
lines changed

7 files changed

+265
-0
lines changed

‎app/code/Magento/Checkout/Model/Session.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
* @api
1818
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1919
* @SuppressWarnings(PHPMD.CookieAndSessionMisuse)
20+
* @SuppressWarnings(PHPMD.TooManyFields)
2021
*/
2122
class Session extends \Magento\Framework\Session\SessionManager
2223
{
@@ -46,6 +47,15 @@ class Session extends \Magento\Framework\Session\SessionManager
4647
*/
4748
protected $_loadInactive = false;
4849

50+
/**
51+
* A flag to track when the quote is being loaded and attached to the session object.
52+
*
53+
* Used in trigger_recollect infinite loop detection.
54+
*
55+
* @var bool
56+
*/
57+
private $isLoading = false;
58+
4959
/**
5060
* Loaded order instance
5161
*
@@ -227,6 +237,10 @@ public function getQuote()
227237
$this->_eventManager->dispatch('custom_quote_process', ['checkout_session' => $this]);
228238

229239
if ($this->_quote === null) {
240+
if ($this->isLoading) {
241+
throw new \LogicException("Infinite loop detected, review the trace for the looping path");
242+
}
243+
$this->isLoading = true;
230244
$quote = $this->quoteFactory->create();
231245
if ($this->getQuoteId()) {
232246
try {
@@ -289,6 +303,7 @@ public function getQuote()
289303

290304
$quote->setStore($this->_storeManager->getStore());
291305
$this->_quote = $quote;
306+
$this->isLoading = false;
292307
}
293308

294309
if (!$this->isQuoteMasked() && !$this->_customerSession->isLoggedIn() && $this->getQuoteId()) {

‎dev/tests/integration/_files/Magento/TestModuleQuoteTotalsObserver/Model/Config.php

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\TestModuleQuoteTotalsObserver\Model;
9+
10+
class Config
11+
{
12+
private $active = false;
13+
14+
public function enableObserver()
15+
{
16+
$this->active = true;
17+
}
18+
19+
public function disableObserver()
20+
{
21+
$this->active = false;
22+
}
23+
24+
public function isActive()
25+
{
26+
return $this->active;
27+
}
28+
}

‎dev/tests/integration/_files/Magento/TestModuleQuoteTotalsObserver/Observer/AfterCollectTotals.php

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\TestModuleQuoteTotalsObserver\Observer;
9+
10+
use Magento\Framework\Event\Observer;
11+
use Magento\Framework\Event\ObserverInterface;
12+
13+
class AfterCollectTotals implements ObserverInterface
14+
{
15+
/**
16+
* @var \Magento\Checkout\Model\Session
17+
*/
18+
private $session;
19+
20+
/**
21+
* @var \Magento\TestModuleQuoteTotalsObserver\Model\Config
22+
*/
23+
private $config;
24+
25+
/**
26+
* AfterCollectTotals constructor.
27+
* @param \Magento\Checkout\Model\Session $messageManager
28+
* @param \Magento\TestModuleQuoteTotalsObserver\Model\Config $config
29+
*/
30+
public function __construct(
31+
\Magento\Checkout\Model\Session $messageManager,
32+
\Magento\TestModuleQuoteTotalsObserver\Model\Config $config
33+
) {
34+
$this->config = $config;
35+
$this->session = $messageManager;
36+
}
37+
38+
/**
39+
* @param Observer $observer
40+
* @return void
41+
*/
42+
public function execute(\Magento\Framework\Event\Observer $observer)
43+
{
44+
$observer->getEvent();
45+
if ($this->config->isActive()) {
46+
$this->session->getQuote();
47+
}
48+
}
49+
}

‎dev/tests/integration/_files/Magento/TestModuleQuoteTotalsObserver/etc/events.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Event/etc/events.xsd">
9+
<event name="sales_quote_collect_totals_after">
10+
<observer name="test_module_quote_totals_observer_after_collect_totals" instance="Magento\TestModuleQuoteTotalsObserver\Observer\AfterCollectTotals" />
11+
</event>
12+
</config>

‎dev/tests/integration/_files/Magento/TestModuleQuoteTotalsObserver/etc/module.xml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0"?>
2+
<!--
3+
/**
4+
* Copyright © Magento, Inc. All rights reserved.
5+
* See COPYING.txt for license details.
6+
*/
7+
-->
8+
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
9+
<module name="Magento_TestModuleQuoteTotalsObserver" active="true">
10+
</module>
11+
</config>

‎dev/tests/integration/_files/Magento/TestModuleQuoteTotalsObserver/registration.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
use Magento\Framework\Component\ComponentRegistrar;
8+
9+
$registrar = new ComponentRegistrar();
10+
if ($registrar->getPath(ComponentRegistrar::MODULE, 'Magento_TestModuleQuoteTotalsObserver') === null) {
11+
ComponentRegistrar::register(ComponentRegistrar::MODULE, 'Magento_TestModuleQuoteTotalsObserver', __DIR__);
12+
}

‎dev/tests/integration/testsuite/Magento/Quote/Model/QuoteInfiniteLoopTest.php

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Quote\Model;
9+
10+
use Magento\TestFramework\Helper\Bootstrap;
11+
use Magento\TestFramework\ObjectManager;
12+
13+
/**
14+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
15+
*/
16+
class QuoteInfiniteLoopTest extends \PHPUnit\Framework\TestCase
17+
{
18+
/**
19+
* @var ObjectManager
20+
*/
21+
private $objectManager;
22+
23+
/**
24+
* @var \Magento\TestModuleQuoteTotalsObserver\Model\Config
25+
*/
26+
private $config;
27+
28+
/**
29+
* @inheritdoc
30+
*/
31+
protected function setUp()
32+
{
33+
$this->objectManager = Bootstrap::getObjectManager();
34+
$this->config = $this->objectManager->get(\Magento\TestModuleQuoteTotalsObserver\Model\Config::class);
35+
$this->config->disableObserver();
36+
}
37+
38+
/**
39+
* @inheritdoc
40+
*/
41+
protected function tearDown()
42+
{
43+
$this->config->disableObserver();
44+
$this->objectManager->removeSharedInstance(\Magento\Checkout\Model\Session::class);
45+
}
46+
47+
/**
48+
* @dataProvider getLoadQuoteParametersProvider
49+
*
50+
* @param $triggerRecollect
51+
* @param $observerEnabled
52+
* @return void
53+
*/
54+
public function testLoadQuoteSuccessfully($triggerRecollect, $observerEnabled): void
55+
{
56+
$originalQuote = $this->generateQuote($triggerRecollect);
57+
$quoteId = $originalQuote->getId();
58+
59+
$this->assertGreaterThan(0, $quoteId, "The quote should have a database id");
60+
$this->assertEquals(
61+
$triggerRecollect,
62+
$originalQuote->getTriggerRecollect(),
63+
"trigger_recollect failed to be set"
64+
);
65+
66+
if ($observerEnabled) {
67+
$this->config->enableObserver();
68+
}
69+
70+
/** @var $session \Magento\Checkout\Model\Session */
71+
$this->objectManager->removeSharedInstance(\Magento\Checkout\Model\Session::class);
72+
$session = $this->objectManager->get(\Magento\Checkout\Model\Session::class);
73+
$session->setQuoteId($quoteId);
74+
75+
$quote = $session->getQuote();
76+
$this->assertEquals($quoteId, $quote->getId(), "The loaded quote should have the same ID as the initial quote");
77+
$this->assertEquals(0, $quote->getTriggerRecollect(), "trigger_recollect should be unset after a quote reload");
78+
}
79+
80+
/**
81+
* @return array
82+
*/
83+
public function getLoadQuoteParametersProvider()
84+
{
85+
return [
86+
[0, false],
87+
[0, true],
88+
[1, false],
89+
//[1, true], this combination of trigger recollect and third party code causes the loop, tested separately
90+
];
91+
}
92+
93+
/**
94+
* @expectedException \LogicException
95+
* @expectedExceptionMessage Infinite loop detected, review the trace for the looping path
96+
*
97+
* @return void
98+
*/
99+
public function testLoadQuoteWithTriggerRecollectInfiniteLoop(): void
100+
{
101+
$originalQuote = $this->generateQuote();
102+
$quoteId = $originalQuote->getId();
103+
104+
$this->assertGreaterThan(0, $quoteId, "The quote should have a database id");
105+
$this->assertEquals(1, $originalQuote->getTriggerRecollect(), "The quote has trigger_recollect set");
106+
107+
// Enable an observer which gets the quote from the session
108+
// The observer hooks into part of the collect totals process for an easy demonstration of the loop.
109+
$this->config->enableObserver();
110+
111+
/** @var $session \Magento\Checkout\Model\Session */
112+
$this->objectManager->removeSharedInstance(\Magento\Checkout\Model\Session::class);
113+
$session = $this->objectManager->get(\Magento\Checkout\Model\Session::class);
114+
$session->setQuoteId($quoteId);
115+
$session->getQuote();
116+
}
117+
118+
/**
119+
* Generate a quote with trigger_recollect and save it in the database.
120+
*
121+
* @param int $triggerRecollect
122+
* @return Quote
123+
*/
124+
private function generateQuote($triggerRecollect = 1)
125+
{
126+
//Fully init a quote with standard quote session procedure
127+
/** @var $session \Magento\Checkout\Model\Session */
128+
$session = $this->objectManager->create(\Magento\Checkout\Model\Session::class);
129+
$session->setQuoteId(null);
130+
$quote = $session->getQuote();
131+
$quote->setTriggerRecollect($triggerRecollect);
132+
133+
/** @var \Magento\Quote\Api\CartRepositoryInterface $quoteRepository */
134+
$quoteRepository = $this->objectManager->create('\Magento\Quote\Api\CartRepositoryInterface');
135+
$quoteRepository->save($quote);
136+
return $quote;
137+
}
138+
}

0 commit comments

Comments
 (0)