Fix issue product name in minicart render wrong with special characters

[mrtuvn]

Description (*)

With product has url magento render name without problem. But in the case product without url will render product name in cart incorrectly. Ex: product with special characters like double quotes
This fix will cover the case products don't have url and render name correctly

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes Minicart product name - not covered cases #29075

Manual testing scenarios (*)

1. From backend admin create new product with name contain special character such as double quote, &, -, /
2. Clear cache go to frontend view new product

Questions or comments

I have one question need clarify about knockout
In template inside foreach loop when use $parent for reference to component i can't call to method that i have defined But if i used $parents[1] its work to call method! Any reason behind this ? i don't see any doc mention in magento official devdocs
Is that knockout context changed inside loop foreach ?
CC: @ihor-sviziev @omiroshnichenko @guz-anton

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @mrtuvn. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[ihor-sviziev]

@magento run all tests

[ihor-sviziev]

@mrtuvn could you fix the failing static tests?

[mrtuvn]

@ihor-sviziev just find out this rule in validate code magento
https://devdocs.magento.com/guides/v2.4/extension-dev-guide/xss-protection.html

In order to notify developers that these properties/function results may contain HTML, Magento requires (with the help of a static test) that you name such properties/functions using “UnsanitizedHtml” suffix.

That is reason of the fail static test
In this specific case i don't think we need touch more on this. Product name already escape html inside php so don't need worry about attacker can inject code in product name

[mrtuvn]

Need your opinions ! @lbajsarowicz @lenaorobei @sivaschenko

[ihor-sviziev]

@mrtuvn I believe we need to ignore these cases as we have html there for sure

[mrtuvn]

What should i do on this ? Not only the product name, still other place in this file fail static test

magento2/app/code/Magento/Checkout/view/frontend/web/template/minicart/item/default.html

Line 48 in 32b520c

<span data-bind="html: option.value"></span>

This line will violation static rule magento about html sanitize

[gabrieldagama]

The risk was set tolow due to: the suggested changes shouldn't affect other areas.

[engcom-Charlie]

@mrtuvn please see https://devdocs.magento.com/guides/v2.4/extension-dev-guide/xss-protection.html about Knockout templates this should help you solve the problem.

[ihor-sviziev]

@mrtuvn from what @engcom-Charlie shown above - seems like we need to change the variable name from product_name to productNameUnsanitizedHtml, revert change that was done in https://github.com/magento/magento2/pull/13802/files#diff-6502b94b418435d6341605ae8a37b46fR88 and add new variable productNameUnsanitizedHtml instead.
Could you do that?

[mrtuvn]

yes i can do that but seem need more effort that still have other variable like options
https://github.com/afirlejczyk/magento2/blob/764f0862f46204f649367728333f40f39509bd7c/app/code/Magento/Checkout/view/frontend/web/template/minicart/item/default.html#L45

this value seem previously passed validator check

[ihor-sviziev]

@mrtuvn can we move it to some method? I believe if that method will be named correctly - it should not complain.

Seems to me that this check was just added quite recently

[ihor-sviziev]

I just analyzed this HTML one time ago - seems like option.value in cases that you changed should contain already escaped html, so it should come from backend.
I would suggest just to remove escaping on frontend and add comment that it should be already escaped on backend.

As result we'll just fix the issue with product_name and will not change anything related to option.value.

What do you thing?

[ihor-sviziev]

@magento run all tests

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8215 has been created to process this Pull Request

[engcom-Alfa]

✔️ QA Passed

Manual testing scenario:

1. Add product with special characters to cart
2. remove ifnot condition (or trigger usage of it in other way) in template

   magento2/app/code/Magento/Checkout/view/frontend/web/template/minicart/item/default.html

   Lines 24 to 32 in 024625e

   	<div class="product-item-details">
   	<strong class="product-item-name">
   	<!-- ko if: product_has_url -->
   	<a data-bind="attr: {href: product_url}, html: product_name"></a>
   	<!-- /ko -->
   	<!-- ko ifnot: product_has_url -->
   	<!-- ko text: product_name --><!-- /ko -->
   	<!-- /ko -->
   	</strong>

3. open minicart

Before: ✖️ incorrectly renderer product name in the cart

After: ✔️ correctly rendered product name in the cart

[m2-assistant]

Hi @mrtuvn, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.