Automattic / VIP-Coding-Standards

PHP_CodeSniffer ruleset to enforce WordPress VIP coding standards.
https://wpvip.com/documentation/how-to-install-php-code-sniffer-for-wordpress-com-vip/
Other
236 stars 40 forks source link

Wrong short tag detection #739

Closed shvlv closed 1 year ago

shvlv commented 1 year ago

Bug Description

It seems the bug was introduced with https://github.com/Automattic/VIP-Coding-Standards/pull/675. The PR adds \WordPressVIPMinimum\Sniffs\Security\ProperEscapingFunctionSniff::$in_short_echo property which is set in the \WordPressVIPMinimum\Sniffs\Security\ProperEscapingFunctionSniff::process_token method. The problem is the property is never reset while the sniff object is cached - https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Files/File.php#L498.

So if the property was set once it has been never reset.

Minimal Code Snippet

https://github.com/shvlv/vipwpcs-bug

vendor/bin/phpcs f1.php f2.php and vendor/bin/phpcs f2.php f1.php provides the different results.

Error Code

I stumbled upon WordPressVIPMinimum.Security.ProperEscapingFunction.notAttrEscAttr.

Environment

Use php -v and composer show to get versions.

Question Answer
PHP version 8.1.13
PHP_CodeSniffer version 3.6.2
VIPCS version 2.3.3

Additional Context (optional)

I believe in_short_echo should be set to false at the start of every process_token invocation.

Tested Against master branch?

GaryJones commented 1 year ago

Thanks for reporting, @shvlv - I had a quick look, and at first glance, it looks like a good catch and a sensible suggested fix.

Marking as needing investigation to dive a little deeper.