Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue] Revert change from 42577bc that introduced side effect in ConfigurableWYSIWYGValidator, function validateConfigured() #39459

Open
2 of 5 tasks
m2-assistant bot opened this issue Dec 10, 2024 · 4 comments · May be fixed by #39444
Labels
Area: Framework Component: Framework/Wysiwyg Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: PR in progress Reported on 2.4.x Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@m2-assistant
Copy link

m2-assistant bot commented Dec 10, 2024

This issue is automatically created based on existing pull request: #39444: Revert change from 42577bc that introduced side effect in ConfigurableWYSIWYGValidator, function validateConfigured()


Description (*)

Commit 42577bc introduced a side effect in validateConfigured() function of ConfigurableWYSIWYGValidator class. validateConfigured() function now permanently modifies allowedTags property with adding/merging ['body', 'html'] to the list of allowed tags on each invocation. This results in an ever growing list of allowed tags for the lifetime of the object of class ConfigurableWYSIWYGValidator.

This hit us really hard since we have some long running PHP processes, that go trough HTML validation, calling validateConfigured() hundred of thousand of times. In the process, allowedTags list grew huge, with an ever increasing size. We saw out PHP process worked increasingly more slow with CPU being at 100% all the time.

Related Pull Requests

Unknown.

Fixed Issues (if relevant)

No reported issue found.

Manual testing scenarios (*)

  1. Create a long running PHP process that calls validateConfigured() multiple times. It will run increasingly more slowly.

Questions or comments

No Questions

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)
  • README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
  • All automated tests passed successfully (all builds are green)
@github-project-automation github-project-automation bot moved this to Ready for Confirmation in Issue Confirmation and Triage Board Dec 10, 2024
@engcom-Bravo engcom-Bravo added Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it Reported on 2.4.x Indicates original Magento version for the Issue report. labels Dec 10, 2024
@m2-community-project m2-community-project bot added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Dec 10, 2024
@engcom-Hotel engcom-Hotel moved this to Ready for Development in High Priority Backlog Dec 10, 2024
@engcom-Hotel engcom-Hotel self-assigned this Dec 12, 2024
@engcom-Hotel
Copy link
Contributor

Hello @bvitnik,

Thanks for the report and collaboration!

We have tried to reproduce the issue in the latest 2.4-develop branch and the issue is reproducible for us, please refer to the below screenshot, we have tried to print the $this->allowedTags from the ConfigurableWYSIWYGValidator class, its clearly shows that body and html tag is repeating:

image

Hence confirming the issue. Thanks for the contribution.

@engcom-Hotel engcom-Hotel added Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Component: Framework/Wysiwyg Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Area: Framework and removed Issue: ready for confirmation labels Dec 12, 2024
@github-jira-sync-bot
Copy link

✅ Jira issue https://jira.corp.adobe.com/browse/AC-13519 is successfully created for this GitHub issue.

Copy link
Author

m2-assistant bot commented Dec 12, 2024

✅ Confirmed by @engcom-Hotel. Thank you for verifying the issue.
Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@bvitnik
Copy link

bvitnik commented Dec 13, 2024

Hi @engcom-Hotel , thank you for acknowledging and reproducing this issue. This was hell of an effort to spot and debug T_T

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Framework Component: Framework/Wysiwyg Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: PR in progress Reported on 2.4.x Indicates original Magento version for the Issue report. Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
Status: Ready for Development
4 participants