Check preg_quote delimiters in array patterns - #6474
Open
arpitjain099 wants to merge 1 commit into
Open
arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
RegularExpressionQuotingRule bails unless the first argument is a Concat, so it only ever looks at a single-string pattern. preg_replace, preg_replace_callback and preg_filter also accept an array of patterns, which is a documented calling convention, and each element is a pattern in its own right. RegularExpressionPatternRule::extractPatterns() already reads that form for the same three functions, so the two regexp rules disagreed on what a pattern argument can look like. Walk the array items and validate each Concat element the same way a single pattern is validated. Other functions in the list take a single pattern, so the array branch is limited to the three that accept one. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
staabm
reviewed
Sep 19, 2026
Comment on lines
+97
to
+115
| $errors = []; | ||
| foreach ($patternArg->items as $item) { | ||
| if (!$item->value instanceof Concat) { | ||
| continue; | ||
| } | ||
|
|
||
| $itemDelimiters = $this->regexExpressionHelper->getPatternDelimiters($item->value, $scope); | ||
| $errors = array_merge($errors, $this->validateQuoteDelimiters($item->value, $scope, $itemDelimiters)); | ||
| } | ||
|
|
||
| return $errors; | ||
| } | ||
|
|
||
| if (!$patternArg instanceof Concat) { | ||
| return []; | ||
| } | ||
|
|
||
| $patternDelimiters = $this->regexExpressionHelper->getPatternDelimiters($normalizedArgs[0]->value, $scope); | ||
| return $this->validateQuoteDelimiters($normalizedArgs[0]->value, $scope, $patternDelimiters); | ||
| $patternDelimiters = $this->regexExpressionHelper->getPatternDelimiters($patternArg, $scope); | ||
| return $this->validateQuoteDelimiters($patternArg, $scope, $patternDelimiters); |
Contributor
There was a problem hiding this comment.
this could be de-duplicated by collecting all concats in a array first and then do the processing
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
2 participants
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.
RegularExpressionQuotingRulebails unless the first argument is aConcat, so it only ever sees a single-string pattern. Butpreg_replace,preg_replace_callbackandpreg_filteralso take an array of patterns, and those calls reach the gate and get dropped.RegularExpressionPatternRule::extractPatterns()already readsgetConstantArrays()for exactly those three functions, so the two regexp rules disagreed about what a pattern argument can be.I added four array cases to
data/preg-quote.php, each the same delimiter mismatch the rule already reports for a bare string. The test passes unchanged on 2.3.x with all four present. A fifth case whose delimiter matches stays quiet, so the branch discriminates rather than firing on every array.One thing to decide: this adds a level 5 error to code that is currently clean. If you would rather it land behind bleeding edge first, say so and I will move it.
Regexp tests 9/9,
make lintclean, phpcs clean on the touched files.