Skip to content

Commit 2dd8be4

Browse files
convenientconvenient
committed
Throw exception to prevent infinite loop caused by third party code.
Third party code combined with `trigger_recollect` can cause infinite loops in some scenarios. The fix needs to be applied to the third party code, but the Magento core could be more explicit with these kinds of errors.
1 parent a474225 commit 2dd8be4

File tree

7 files changed

+263
-0
lines changed

7 files changed

+263
-0
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ class Session extends \Magento\Framework\Session\SessionManager
4141
*/
4242
protected $_loadInactive = false;
4343

44+
/**
45+
* A flag to track when the quote is being loaded and attached to the session object.
46+
*
47+
* Used in trigger_recollect infinite loop detection.
48+
*
49+
* @var bool
50+
*/
51+
protected $_isLoading = false;
52+
4453
/**
4554
* Loaded order instance
4655
*
@@ -210,6 +219,10 @@ public function getQuote()
210219
$this->_eventManager->dispatch('custom_quote_process', ['checkout_session' => $this]);
211220

212221
if ($this->_quote === null) {
222+
if ($this->_isLoading) {
223+
throw new \LogicException("Infinite loop detected, review the trace for the looping path");
224+
}
225+
$this->_isLoading = true;
213226
$quote = $this->quoteFactory->create();
214227
if ($this->getQuoteId()) {
215228
try {
@@ -262,6 +275,7 @@ public function getQuote()
262275

263276
$quote->setStore($this->_storeManager->getStore());
264277
$this->_quote = $quote;
278+
$this->_isLoading = false;
265279
}
266280

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

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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+
}
29+

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
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(\Magento\Checkout\Model\Session $messageManager, \Magento\TestModuleQuoteTotalsObserver\Model\Config $config)
31+
{
32+
$this->config = $config;
33+
$this->session = $messageManager;
34+
}
35+
36+
/**
37+
* @param Observer $observer
38+
* @return void
39+
*/
40+
public function execute(\Magento\Framework\Event\Observer $observer)
41+
{
42+
if ($this->config->isActive()) {
43+
$this->session->getQuote();
44+
}
45+
}
46+
}
47+

‎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 $thirdPartObserverEnabled
52+
* @return void
53+
*/
54+
public function testLoadQuoteSuccessfully($triggerRecollect, $thirdPartObserverEnabled): 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 ($thirdPartObserverEnabled) {
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)