Skip to content

Avoid infinite loop when Parse Error occurs #202

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

Merged
merged 2 commits into from
Jul 26, 2021

Conversation

fredden
Copy link
Member

@fredden fredden commented Feb 3, 2021

One of our developers introduced a Parse Error into a template file. When phpcs --standard=Magento2 ran as part of our automated testing process, it began using 100% CPU and never exited (until it was terminated automatically for taking too long). Investigation shows that the XSS Template sniff from this repository is at fault. I've managed to narrow down the cause and produced a testcase to reproduce this scenario. The proposed fix checks the return code of PHP_CodeSniffer\Files\File::findNext(), rather than blindly assuming this is an integer. I've not searched for other uses of this function within this codebase.

Copy link

@riconeitzel riconeitzel left a comment

Choose a reason for hiding this comment

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

looks good

@ihor-sviziev
Copy link
Collaborator

ihor-sviziev commented Mar 17, 2021

@sivaschenko @gabrieldagama @sidolov , could you take a look? This change looks good to me

@sivaschenko
Copy link
Member

@magento import pr to magento-commerce/magento-coding-standard

@magento-engcom-team
Copy link
Contributor

@sivaschenko the pull request successfully imported.

@magento-devops-reposync-svc magento-devops-reposync-svc merged commit fa8830c into magento:develop Jul 26, 2021
@fredden fredden deleted the no-infinite-loop branch July 26, 2021 23:43
magento-devops-reposync-svc pushed a commit that referenced this pull request Feb 1, 2023
AC-7822:Phpstan dependency should not be limited to a specific versio…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants