Repository navigation
Flag restricted array members accessed with double-quoted keys - #908
Merged
Merged
Conversation
RestrictedVariables never flagged `$_SERVER["REMOTE_ADDR"]` or `$_SERVER["HTTP_USER_AGENT"]`, only their single-quoted forms. The quote normalisation in test_patterns() used `[` and `]` as regex delimiters rather than as a character class, so it only ever matched a literal `'"` pair and never ran. Even when fixed as intended, rewriting the pattern's quotes to single quotes would not help, because the code being checked is what uses the double quotes. Quotes in a pattern now match either quote style. The ruleset fixtures already contained `$_SERVER["REMOTE_ADDR"]` under ServerVariables, which now also correctly raises the RestrictedVariables warning.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WordPressVIPMinimum.Variables.RestrictedVariableswarns about$_SERVER['REMOTE_ADDR']and$_SERVER['HTTP_USER_AGENT'], but says nothing about$_SERVER["REMOTE_ADDR"]or{$_SERVER["HTTP_USER_AGENT"]}in a heredoc. They're the same variables, so the cache constraints warning should apply to both. I spotted this while reviewing #901.The cause is in
test_patterns(), which turns each configured pattern into a regex. Its second replacement,'[\'"]', was meant to be a character class matching either quote, but PCRE reads the[and]as the delimiters. It only ever matched a literal'"pair, so it never did anything. Fixing the delimiters alone wouldn't help either: rewriting the pattern's quotes to single quotes changes nothing when the patterns are already single-quoted and it's the code being checked that uses double quotes.So quotes in a pattern now become
['"], matching either quote style in the code. This applies to every sniff built onAbstractVariableRestrictionsSniff, which within VIPCS is justRestrictedVariables.Sites with double-quoted
$_SERVER["REMOTE_ADDR"]or$_SERVER["HTTP_USER_AGENT"]will start seeing the warning they should always have had. Both ruleset fixtures already contained$_SERVER["REMOTE_ADDR"]under the ServerVariables section, and those lines now expect the extra warning too.