Skip to content

Commit

Permalink
Bugfix: Render script in secure tag #797
Browse files Browse the repository at this point in the history
  • Loading branch information
@michielgerritsen
michielgerritsen committed Jul 23, 2024
1 parent 49565b2 commit e3c7f9c
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 26 deletions.
12 changes: 9 additions & 3 deletions view/adminhtml/templates/form/mollie_paymentlink_javascript.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
* Copyright Magmodules.eu. All rights reserved.
* See COPYING.txt for license details.
*/
?>
<script>

use Magento\Framework\View\Helper\SecureHtmlRenderer;

/** @var SecureHtmlRenderer $secureRenderer */

$scriptString = <<<SCRIPT
document.addEventListener('DOMContentLoaded', function () {
const saveSelectedMethods = () => {
// Save the selected payment methods to local storage
Expand Down Expand Up @@ -46,4 +50,6 @@
setSelectedMethods();
})
});
</script>
SCRIPT;

echo $secureRenderer->renderTag('script', [], $scriptString, false);
16 changes: 13 additions & 3 deletions view/adminhtml/templates/order/shipment/create/payment_hold_warning.phtml
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
<?php
/*
* Copyright Magmodules.eu. All rights reserved.
* See COPYING.txt for license details.
*/

declare(strict_types=1);
?>

use Magento\Framework\View\Helper\SecureHtmlRenderer;

/** @var SecureHtmlRenderer $secureRenderer */
?>
<div class="mollie-manual-capture-warning message message-warning">
Please note: You are creating a partial shipment, but it's only possible to capture the payment once.
Please double-check you are shipping the correct items.
</div>

<script>
<?php
$scriptString = <<<SCRIPT
(() => {
let warningElement = document.querySelector('.mollie-manual-capture-warning');
let fields = Array.from(document.querySelectorAll('.qty-item'));
Expand All @@ -28,4 +36,6 @@ declare(strict_types=1);
checkFields();
})();
</script>
SCRIPT;

echo $secureRenderer->renderTag('script', [], $scriptString, false);
25 changes: 16 additions & 9 deletions view/adminhtml/templates/system/config/button/apikey.phtml
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
<?php
/**
* Copyright © 2018 Magmodules.eu. All rights reserved.
/*
* Copyright Magmodules.eu. All rights reserved.
* See COPYING.txt for license details.
*/

use Magento\Framework\View\Helper\SecureHtmlRenderer;
use Mollie\Payment\Block\Adminhtml\System\Config\Form\Apikey\Checker;

/**
* @see \Mollie\Payment\Block\Adminhtml\System\Config\Form\Apikey\Checker
* @var \Mollie\Payment\Block\Adminhtml\System\Config\Form\Apikey\Checker $block
* @see Checker
* @var Checker $block
* @var SecureHtmlRenderer $secureRenderer
*/
?>
<script>

$scriptString = <<<SCRIPT
require([
'jquery',
'prototype'
Expand All @@ -20,7 +24,7 @@
"test_key": jQuery("#mollie_general_api_details_apikey_test").val(),
"live_key": jQuery("#mollie_general_api_details_apikey_live").val()
};
new Ajax.Request('<?= $block->getAjaxUrl() ?>', {
new Ajax.Request('{$block->getAjaxUrl()}', {
parameters: params,
loaderArea: false,
asynchronous: true,
Expand All @@ -45,5 +49,8 @@
});
});
});
</script>
<?= $block->getButtonHtml() ?>
SCRIPT;

echo $secureRenderer->renderTag('script', [], $scriptString, false);

echo $block->getButtonHtml();
29 changes: 18 additions & 11 deletions view/adminhtml/templates/system/config/button/compatibility.phtml
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
<?php
/**
* Copyright © 2018 Magmodules.eu. All rights reserved.
/*
* Copyright Magmodules.eu. All rights reserved.
* See COPYING.txt for license details.
*/

use Magento\Framework\View\Helper\SecureHtmlRenderer;
use Mollie\Payment\Block\Adminhtml\System\Config\Form\Compatibility\Checker;

/**
* @see \Mollie\Payment\Block\Adminhtml\System\Config\Form\Compatibility\Checker
* @var \Mollie\Payment\Block\Adminhtml\System\Config\Form\Compatibility\Checker $block
* @see Checker
* @var Checker $block
* @var SecureHtmlRenderer $secureRenderer
*/
?>
<script>

$scriptString = <<<SCRIPT
require([
'jquery',
'mage/translate',
'prototype',
], function (jQuery, $t) {
], function (jQuery, \$t) {
var resultSpan = jQuery('#result_compatibility');
jQuery('#compatibility_button').click(function () {
var params = {};
new Ajax.Request('<?= $block->getAjaxUrl() ?>', {
new Ajax.Request('{$block->getAjaxUrl()}', {
parameters: params,
loaderArea: false,
asynchronous: true,
Expand All @@ -36,7 +40,7 @@
if (typeof json.msg != 'undefined') {
resultText = json.msg;
} else {
resultText = $t('Invalid response received. This indicates an unknown problem.');
resultText = \$t('Invalid response received. This indicates an unknown problem.');
}
}
resultSpan.find('.result').show();
Expand All @@ -46,5 +50,8 @@
});
});
</script>
<?= $block->getButtonHtml() ?>
SCRIPT;

echo $secureRenderer->renderTag('script', [], $scriptString, false);

echo $block->getButtonHtml();

4 comments on commit e3c7f9c

@hostep
Copy link

@hostep hostep commented on e3c7f9c Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michielgerritsen, your module claims to be compatible with Magento 2.3.3 and higher. But the Magento\Framework\View\Helper\SecureHtmlRenderer class was only introduced in Magento 2.4.0, so either you'll need to check if it exists before trying to use it, or drop support for Magento 2.3.x, otherwise this code will crash on Magento 2.3.x shops.

Also, there is a bug in Magento < 2.4.0 when using heredoc syntax (like your <<<SCRIPT) that breaks when html minification is enabled. That was also fixed in Magento 2.4.0 and higher.

See discussions in yireo/Yireo_GoogleTagManager2#231 & algolia/algoliasearch-magento-2#1541 for where similar issues were discussed in other open source modules.

@hostep
Copy link

@hostep hostep commented on e3c7f9c Aug 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange that your automated tests don't pick this up...

Running phpstan locally with level 1 on a Magento 2.3.7-p4 shop gives these problems:

$ vendor/bin/phpstan analyse vendor/mollie/magento2
Note: Using configuration file phpstan.neon.
 470/470 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -------------------------------------------------------------------
  Line   view/adminhtml/templates/form/mollie_paymentlink_javascript.phtml
 ------ -------------------------------------------------------------------
  55     Undefined variable: $secureRenderer
 ------ -------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------
  Line   view/adminhtml/templates/order/shipment/create/payment_hold_warning.phtml
 ------ ---------------------------------------------------------------------------
  41     Undefined variable: $secureRenderer
 ------ ---------------------------------------------------------------------------

 ------ ------------------------------------------------------------
  Line   view/adminhtml/templates/system/config/button/apikey.phtml
 ------ ------------------------------------------------------------
  54     Undefined variable: $secureRenderer
 ------ ------------------------------------------------------------

 ------ -------------------------------------------------------------------
  Line   view/adminhtml/templates/system/config/button/compatibility.phtml
 ------ -------------------------------------------------------------------
  55     Undefined variable: $secureRenderer
 ------ -------------------------------------------------------------------

If we increase phpstan to level 2, the problems becomes clearer:

$ vendor/bin/phpstan analyse vendor/mollie/magento2
Note: Using configuration file phpstan.neon.
 470/470 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -----------------------------------------------------------------------------------------------------------------------
  Line   view/adminhtml/templates/form/mollie_paymentlink_javascript.phtml
 ------ -----------------------------------------------------------------------------------------------------------------------
  11     PHPDoc tag @var for variable $secureRenderer contains unknown class Magento\Framework\View\Helper\SecureHtmlRenderer.
  11     Variable $secureRenderer in PHPDoc tag @var does not match assigned variable $scriptString.
  55     Undefined variable: $secureRenderer
 ------ -----------------------------------------------------------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------
  Line   view/adminhtml/templates/order/shipment/create/payment_hold_warning.phtml
 ------ ---------------------------------------------------------------------------
  41     Undefined variable: $secureRenderer
 ------ ---------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------------------------------
  Line   view/adminhtml/templates/system/config/button/apikey.phtml
 ------ -----------------------------------------------------------------------------------------------------------------------
  16     Multiple PHPDoc @var tags above single variable assignment are not supported.
  16     PHPDoc tag @var for variable $secureRenderer contains unknown class Magento\Framework\View\Helper\SecureHtmlRenderer.
  54     Undefined variable: $secureRenderer
 ------ -----------------------------------------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------------------------------
  Line   view/adminhtml/templates/system/config/button/compatibility.phtml
 ------ -----------------------------------------------------------------------------------------------------------------------
  16     Multiple PHPDoc @var tags above single variable assignment are not supported.
  16     PHPDoc tag @var for variable $secureRenderer contains unknown class Magento\Framework\View\Helper\SecureHtmlRenderer.
  55     Undefined variable: $secureRenderer
 ------ -----------------------------------------------------------------------------------------------------------------------

@michielgerritsen
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hostep Awesome feedback, thanks! The code has been updated.

It looks like the PHPStan check isn't working as expected. I have noted this and will look into it later.

@Frank-Magmodules
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed; Thank you so much for taking the time and effort to do this, @hostep. It’s truly helpful!

Please sign in to comment.