Skip to content

Commit e4da7ad

Browse files
nathanjosiahnathanjosiah
authored
Merge pull request #5203 from magento-borg/borg-2.4.0
[CIA] Bug fixes
2 parents 5ad40fa + ef591d2 commit e4da7ad

File tree

22 files changed

+261
-52
lines changed

22 files changed

+261
-52
lines changed

‎app/code/Magento/Braintree/Test/Mftf/ActionGroup/AdminDeleteRoleActionGroup.xml

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,32 @@
1010
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
1111
<actionGroup name="AdminDeleteRoleActionGroup">
1212
<annotations>
13-
<description>Deletes a User Role that contains the text 'Role'. PLEASE NOTE: The Action Group values are Hardcoded.</description>
13+
<description>Deletes a User Role.</description>
1414
</annotations>
1515
<arguments>
1616
<argument name="role" defaultValue=""/>
1717
</arguments>
1818

19-
<click stepKey="clickOnRole" selector="{{AdminDeleteRoleSection.theRole}}"/>
19+
<click stepKey="clickResetFilterButtonBefore" selector="{{AdminRoleGridSection.resetButton}}"/>
20+
<waitForPageLoad stepKey="waitForRolesGridFilterResetBefore" time="10"/>
21+
<fillField stepKey="TypeRoleFilter" selector="{{AdminRoleGridSection.roleNameFilterTextField}}" userInput="{{role.name}}"/>
22+
<waitForElementVisible stepKey="waitForFilterSearchButtonBefore" selector="{{AdminRoleGridSection.searchButton}}" time="10"/>
23+
<click stepKey="clickFilterSearchButton" selector="{{AdminRoleGridSection.searchButton}}"/>
24+
<waitForPageLoad stepKey="waitForUserRoleFilter" time="10"/>
25+
<waitForElementVisible stepKey="waitForRoleInRoleGrid" selector="{{AdminDeleteRoleSection.role(role.name)}}" time="10"/>
26+
<click stepKey="clickOnRole" selector="{{AdminDeleteRoleSection.role(role.name)}}"/>
27+
<waitForPageLoad stepKey="waitForRolePageToLoad" time="10"/>
2028
<fillField stepKey="TypeCurrentPassword" selector="{{AdminDeleteRoleSection.current_pass}}" userInput="{{_ENV.MAGENTO_ADMIN_PASSWORD}}"/>
29+
<waitForElementVisible stepKey="waitForDeleteRoleButton" selector="{{AdminDeleteRoleSection.delete}}" time="10"/>
2130
<click stepKey="clickToDeleteRole" selector="{{AdminDeleteRoleSection.delete}}"/>
22-
<waitForAjaxLoad stepKey="waitForDeleteConfirmationPopup" time="5"/>
31+
<waitForPageLoad stepKey="waitForDeleteConfirmationPopup" time="5"/>
32+
<waitForElementVisible stepKey="waitForConfirmButton" selector="{{AdminDeleteRoleSection.confirm}}" time="10"/>
2333
<click stepKey="clickToConfirm" selector="{{AdminDeleteRoleSection.confirm}}"/>
2434
<waitForPageLoad stepKey="waitForPageLoad" time="10"/>
2535
<see stepKey="seeSuccessMessage" userInput="You deleted the role."/>
36+
<waitForPageLoad stepKey="waitForRolesGridLoad" />
37+
<waitForElementVisible stepKey="waitForResetFilterButtonAfter" selector="{{AdminRoleGridSection.resetButton}}" time="10"/>
38+
<click stepKey="clickResetFilterButtonAfter" selector="{{AdminRoleGridSection.resetButton}}"/>
39+
<waitForPageLoad stepKey="waitForRolesGridFilterResetAfter" time="10"/>
2640
</actionGroup>
2741
</actionGroups>

‎app/code/Magento/CardinalCommerce/Model/JwtManagement.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace Magento\CardinalCommerce\Model;
99

1010
use Magento\Framework\Serialize\Serializer\Json;
11+
use Magento\Framework\Encryption\Helper\Security;
1112

1213
/**
1314
* JSON Web Token management.
@@ -62,7 +63,8 @@ public function decode(string $jwt, string $key): array
6263
$payload = $this->json->unserialize($payloadJson);
6364

6465
$signature = $this->urlSafeB64Decode($signatureB64);
65-
if ($signature !== $this->sign($headB64 . '.' . $payloadB64, $key, $header['alg'])) {
66+
67+
if (!Security::compareStrings($signature, $this->sign($headB64 . '.' . $payloadB64, $key, $header['alg']))) {
6668
throw new \InvalidArgumentException('JWT signature verification failed');
6769
}
6870

‎app/code/Magento/Catalog/Block/Product/ListProduct.php

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -346,17 +346,16 @@ public function getIdentities()
346346

347347
$category = $this->getLayer()->getCurrentCategory();
348348
if ($category) {
349-
$identities[] = Product::CACHE_PRODUCT_CATEGORY_TAG . '_' . $category->getId();
349+
$identities[] = [Product::CACHE_PRODUCT_CATEGORY_TAG . '_' . $category->getId()];
350350
}
351351

352352
//Check if category page shows only static block (No products)
353-
if ($category->getData('display_mode') == Category::DM_PAGE) {
354-
return $identities;
355-
}
356-
357-
foreach ($this->_getProductCollection() as $item) {
358-
$identities = array_merge($identities, $item->getIdentities());
353+
if ($category->getData('display_mode') != Category::DM_PAGE) {
354+
foreach ($this->_getProductCollection() as $item) {
355+
$identities[] = $item->getIdentities();
356+
}
359357
}
358+
$identities = array_merge(...$identities);
360359

361360
return $identities;
362361
}
@@ -369,7 +368,7 @@ public function getIdentities()
369368
*/
370369
public function getAddToCartPostParams(Product $product)
371370
{
372-
$url = $this->getAddToCartUrl($product);
371+
$url = $this->getAddToCartUrl($product, ['_escape' => false]);
373372
return [
374373
'action' => $url,
375374
'data' => [

‎app/code/Magento/Catalog/Test/Unit/Block/Product/ListProductTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ public function testGetAddToCartPostParams()
217217
->will($this->returnValue(true));
218218
$this->cartHelperMock->expects($this->any())
219219
->method('getAddUrl')
220-
->with($this->equalTo($this->productMock), $this->equalTo([]))
220+
->with($this->equalTo($this->productMock), $this->equalTo(['_escape' => false]))
221221
->will($this->returnValue($url));
222222
$this->productMock->expects($this->once())
223223
->method('getEntityId')

‎app/code/Magento/Cms/Model/Template/Filter.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
*/
66
namespace Magento\Cms\Model\Template;
77

8-
use Magento\Framework\Exception\LocalizedException;
9-
108
/**
119
* Cms Template Filter Model
1210
*/
@@ -41,8 +39,8 @@ public function mediaDirective($construction)
4139
{
4240
// phpcs:ignore Magento2.Functions.DiscouragedFunction
4341
$params = $this->getParameters(html_entity_decode($construction[2], ENT_QUOTES));
44-
if (preg_match('/\.\.(\\\|\/)/', $params['url'])) {
45-
throw new \InvalidArgumentException('Image path must be absolute');
42+
if (preg_match('/(^.*:\/\/.*|\.\.\/.*)/', $params['url'])) {
43+
throw new \InvalidArgumentException('Image path must be absolute and not include URLs');
4644
}
4745

4846
return $this->_storeManager->getStore()->getBaseMediaDir() . '/' . $params['url'];

‎app/code/Magento/Cms/Test/Unit/Model/Template/FilterTest.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,4 +105,24 @@ public function testMediaDirectiveRelativePath()
105105
->willReturn($baseMediaDir);
106106
$this->filter->mediaDirective($construction);
107107
}
108+
109+
/**
110+
* Test using media directive with a URL path including schema.
111+
*
112+
* @covers \Magento\Cms\Model\Template\Filter::mediaDirective
113+
* @expectedException \InvalidArgumentException
114+
*/
115+
public function testMediaDirectiveURL()
116+
{
117+
$baseMediaDir = 'pub/media';
118+
$construction = [
119+
'{{media url="http://wysiwyg/images/image.jpg"}}',
120+
'media',
121+
' url="http://wysiwyg/images/../image.jpg"'
122+
];
123+
$this->storeMock->expects($this->any())
124+
->method('getBaseMediaDir')
125+
->willReturn($baseMediaDir);
126+
$this->filter->mediaDirective($construction);
127+
}
108128
}

‎app/code/Magento/Customer/view/frontend/templates/js/customer-data.phtml

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,21 @@
55
*/
66

77
/** @var \Magento\Customer\Block\CustomerData $block */
8+
9+
// phpcs:disable Magento2.Templates.ThisInTemplate.FoundHelper
810
?>
911
<script type="text/x-magento-init">
1012
{
1113
"*": {
1214
"Magento_Customer/js/customer-data": {
13-
"sectionLoadUrl": "<?= $block->escapeJs($block->escapeUrl($block->getCustomerDataUrl('customer/section/load'))) ?>",
15+
"sectionLoadUrl": "<?= $block->escapeJs($block->getCustomerDataUrl('customer/section/load')) ?>",
1416
"expirableSectionLifetime": <?= (int)$block->getExpirableSectionLifetime() ?>,
15-
"expirableSectionNames": <?= /* @noEscape */ $this->helper(\Magento\Framework\Json\Helper\Data::class)->jsonEncode($block->getExpirableSectionNames()) ?>,
17+
"expirableSectionNames": <?= /* @noEscape */ $this->helper(\Magento\Framework\Json\Helper\Data::class)
18+
->jsonEncode($block->getExpirableSectionNames()) ?>,
1619
"cookieLifeTime": "<?= $block->escapeJs($block->getCookieLifeTime()) ?>",
17-
"updateSessionUrl": "<?= $block->escapeJs($block->escapeUrl($block->getCustomerDataUrl('customer/account/updateSession'))) ?>"
20+
"updateSessionUrl": "<?= $block->escapeJs(
21+
$block->getCustomerDataUrl('customer/account/updateSession')
22+
) ?>"
1823
}
1924
}
2025
}

‎app/code/Magento/Directory/Block/Data.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public function getCountryHtmlSelect($defValue = null, $name = 'country_id', $id
142142
)->setId(
143143
$id
144144
)->setTitle(
145-
__($title)
145+
$this->escapeHtmlAttr(__($title))
146146
)->setValue(
147147
$defValue
148148
)->setOptions(

‎app/code/Magento/Directory/Test/Unit/Block/DataTest.php

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Magento\Store\Model\ScopeInterface;
1818
use Magento\Store\Model\Store;
1919
use Magento\Store\Model\StoreManagerInterface;
20+
use Magento\Framework\Escaper;
2021

2122
/**
2223
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -56,9 +57,16 @@ class DataTest extends \PHPUnit\Framework\TestCase
5657
/** @var SerializerInterface|\PHPUnit_Framework_MockObject_MockObject */
5758
private $serializerMock;
5859

60+
/** @var \Magento\Framework\Escaper */
61+
private $escaper;
62+
5963
protected function setUp()
6064
{
6165
$objectManagerHelper = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
66+
$this->escaper = $this->getMockBuilder(Escaper::class)
67+
->disableOriginalConstructor()
68+
->setMethods(['escapeHtmlAttr'])
69+
->getMock();
6270
$this->prepareContext();
6371

6472
$this->helperDataMock = $this->getMockBuilder(\Magento\Directory\Helper\Data::class)
@@ -123,6 +131,10 @@ protected function prepareContext()
123131
$this->contextMock->expects($this->any())
124132
->method('getLayout')
125133
->willReturn($this->layoutMock);
134+
135+
$this->contextMock->expects($this->once())
136+
->method('getEscaper')
137+
->willReturn($this->escaper);
126138
}
127139

128140
protected function prepareCountryCollection()
@@ -135,9 +147,11 @@ protected function prepareCountryCollection()
135147
\Magento\Directory\Model\ResourceModel\Country\CollectionFactory::class
136148
)
137149
->disableOriginalConstructor()
138-
->setMethods([
139-
'create'
140-
])
150+
->setMethods(
151+
[
152+
'create'
153+
]
154+
)
141155
->getMock();
142156

143157
$this->countryCollectionFactoryMock->expects($this->any())
@@ -285,15 +299,17 @@ protected function mockElementHtmlSelect($defaultCountry, $options, $resultHtml)
285299

286300
$elementHtmlSelect = $this->getMockBuilder(\Magento\Framework\View\Element\Html\Select::class)
287301
->disableOriginalConstructor()
288-
->setMethods([
289-
'setName',
290-
'setId',
291-
'setTitle',
292-
'setValue',
293-
'setOptions',
294-
'setExtraParams',
295-
'getHtml',
296-
])
302+
->setMethods(
303+
[
304+
'setName',
305+
'setId',
306+
'setTitle',
307+
'setValue',
308+
'setOptions',
309+
'setExtraParams',
310+
'getHtml',
311+
]
312+
)
297313
->getMock();
298314

299315
$elementHtmlSelect->expects($this->once())
@@ -323,6 +339,10 @@ protected function mockElementHtmlSelect($defaultCountry, $options, $resultHtml)
323339
$elementHtmlSelect->expects($this->once())
324340
->method('getHtml')
325341
->willReturn($resultHtml);
342+
$this->escaper->expects($this->once())
343+
->method('escapeHtmlAttr')
344+
->with(__($title))
345+
->willReturn(__($title));
326346

327347
return $elementHtmlSelect;
328348
}

‎app/code/Magento/Email/Model/Template/Filter.php

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
namespace Magento\Email\Model\Template;
77

88
use Magento\Framework\App\ObjectManager;
9+
use Magento\Framework\Exception\MailException;
10+
use Magento\Framework\Exception\NoSuchEntityException;
911
use Magento\Framework\Filesystem;
10-
use Magento\Framework\Filesystem\Directory\ReadInterface;
1112
use Magento\Framework\Filter\VariableResolverInterface;
1213
use Magento\Framework\View\Asset\ContentProcessorException;
1314
use Magento\Framework\View\Asset\ContentProcessorInterface;
@@ -734,27 +735,32 @@ public function modifierEscape($value, $type = 'html')
734735
* {{protocol store="1"}} - Optional parameter which gets protocol from provide store based on store ID or code
735736
*
736737
* @param string[] $construction
737-
* @throws \Magento\Framework\Exception\MailException
738738
* @return string
739+
* @throws MailException
740+
* @throws NoSuchEntityException
739741
*/
740742
public function protocolDirective($construction)
741743
{
742744
$params = $this->getParameters($construction[2]);
745+
743746
$store = null;
744747
if (isset($params['store'])) {
745748
try {
746749
$store = $this->_storeManager->getStore($params['store']);
747750
} catch (\Exception $e) {
748-
throw new \Magento\Framework\Exception\MailException(
751+
throw new MailException(
749752
__('Requested invalid store "%1"', $params['store'])
750753
);
751754
}
752755
}
756+
753757
$isSecure = $this->_storeManager->getStore($store)->isCurrentlySecure();
754758
$protocol = $isSecure ? 'https' : 'http';
755759
if (isset($params['url'])) {
756760
return $protocol . '://' . $params['url'];
757761
} elseif (isset($params['http']) && isset($params['https'])) {
762+
$this->validateProtocolDirectiveHttpScheme($params);
763+
758764
if ($isSecure) {
759765
return $params['https'];
760766
}
@@ -764,6 +770,37 @@ public function protocolDirective($construction)
764770
return $protocol;
765771
}
766772

773+
/**
774+
* Validate protocol directive HTTP parameters.
775+
*
776+
* @param string[] $params
777+
* @return void
778+
* @throws MailException
779+
*/
780+
private function validateProtocolDirectiveHttpScheme(array $params) : void
781+
{
782+
$parsed_http = parse_url($params['http']);
783+
$parsed_https = parse_url($params['https']);
784+
785+
if (empty($parsed_http)) {
786+
throw new MailException(
787+
__('Contents of %1 could not be loaded or is empty', $params['http'])
788+
);
789+
} elseif (empty($parsed_https)) {
790+
throw new MailException(
791+
__('Contents of %1 could not be loaded or is empty', $params['https'])
792+
);
793+
} elseif ($parsed_http['scheme'] !== 'http') {
794+
throw new MailException(
795+
__('Contents of %1 could not be loaded or is empty', $params['http'])
796+
);
797+
} elseif ($parsed_https['scheme'] !== 'https') {
798+
throw new MailException(
799+
__('Contents of %1 could not be loaded or is empty', $params['https'])
800+
);
801+
}
802+
}
803+
767804
/**
768805
* Store config directive
769806
*

0 commit comments

Comments
 (0)