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

RestrictedFunctions: fix false negative - functions in config_settings would never match #566

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Jul 28, 2020

The functions in the config_settings group were using a unicode _, not the ascii _, so these would never match the PHP native opcache_* functions they were intended to match.

As the same unicode character was used in the applicable unit tests and ruleset tests, the bug was never reported as a test failure.

Related to #518 (sniff review)

@jrfnl jrfnl added this to the 2.2.0 milestone Jul 28, 2020
@jrfnl jrfnl requested a review from a team as a code owner July 28, 2020 10:24
@jrfnl jrfnl force-pushed the fix/restricted-functions-opcache-config-settings-false-negative branch from dd45ecd to b61a257 Compare July 28, 2020 10:47
…gs` would never match

The functions in the `config_settings` group were using a unicode `_`, not the ascii `_`, so these would never match the PHP native `opcache_*` functions they were intended to match.

As the same unicode character was used in the applicable unit tests and ruleset tests, the bug was never reported as a test failure.
Copy link
Contributor

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

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

Eeek - that's an amazing catch!

@GaryJones GaryJones merged commit ad76879 into develop Jul 28, 2020
@GaryJones GaryJones deleted the fix/restricted-functions-opcache-config-settings-false-negative branch July 28, 2020 12:32
@rebeccahum
Copy link
Contributor

Nice!

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.

3 participants