Skip to content

Add semicolon as statement separator in the special annotation check of the Magento2.Security.XssTemplate sniff #378

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

Conversation

aadmathijssen
Copy link
Contributor

This PR improves special annotation detection of the Magento2.Security.XssTemplate sniff by adding T_SEMICOLON as a statement separator. This resolves the issue that violations in a statement might be ignored by a special annotation in a previous/subsequent statement.

For example, consider the following file:

<?php
    /* @noEscape */ echo $foo;
    echo $bar;
?>
<?=
    $baz;
    /* @noEscape */ echo $qux;
?>

This does not reported any violations of the Magento2.Security.XssTemplate sniff.

However, the statements involving $bar and $baz should report a FoundUnescaped violation.

This PR makes sure this is done, such that the following is reported:

FILE: /path/to/file.phtml
-----------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------
 3 | WARNING | Unescaped output detected.
   |         | (Magento2.Security.XssTemplate.FoundUnescaped)
 6 | WARNING | Unescaped output detected.
   |         | (Magento2.Security.XssTemplate.FoundUnescaped)
-----------------------------------------------------------------------------------------

…of the `Magento2.Security.XssTemplate` sniff
Copy link
Member

@fredden fredden left a comment

Choose a reason for hiding this comment

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

The code changes look good to me. Please can you add a test case to cover this change.

Copy link
Collaborator

@ihor-sviziev ihor-sviziev left a comment

Choose a reason for hiding this comment

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

Please add test coverage for your changes

@sidolov
Copy link
Collaborator

sidolov commented Sep 20, 2023

@magento import PR to magento-commerce/magento-coding-standard repository

@m2-github-services
Copy link
Contributor

@sidolov the Pull Request is successfully imported.

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