minor #58261 [ExpressionLanguage] Fix matches to handle booleans being used as regexp (ivantsepp)

fabpot · fabpot · commit 128efbb1fd16 · 2024-10-06T11:28:55.000+02:00
This PR was squashed before being merged into the 7.2 branch. Discussion ---------- [ExpressionLanguage] Fix matches to handle booleans being used as regexp | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | n/a | License | MIT I'm marking this as a bug fix but this could be seen as a "new feature" as existing `matches` syntax functionality works just fine. I wanted to build on top of the work done at #45875. This handles invalid regular expressions and also does an extra check inside the `compile` method. It checks the right hand side for a `ConstantNode` and validates that it's a valid regexp. The idea is that we can go **even further** and check for `BinaryNode` because in most cases, this is an invalid regexp since a boolean usually returned. The exception is `~` which is for string concatenation since that could result in a valid regexp. This extra check could help prevent invalid expressions like `"a" matches ("/a/" || "/b/")` where one could mistake `"/a/" || "/b/"` as being a valid regexp (when the correct approach would've been `"a" matches "/a|b/"`) Commits ------- e0fbc7e [ExpressionLanguage] Fix matches to handle booleans being used as regexp
diff --git a/src/Symfony/Component/ExpressionLanguage/Node/BinaryNode.php b/src/Symfony/Component/ExpressionLanguage/Node/BinaryNode.php
@@ -52,6 +52,8 @@ public function compile(Compiler $compiler): void
         if ('matches' == $operator) {
             if ($this->nodes['right'] instanceof ConstantNode) {
                 $this->evaluateMatches($this->nodes['right']->evaluate([], []), '');
+            } elseif ($this->nodes['right'] instanceof self && '~' !== $this->nodes['right']->attributes['operator']) {
+                throw new SyntaxError('The regex passed to "matches" must be a string.');
             }
 
             $compiler
diff --git a/src/Symfony/Component/ExpressionLanguage/Tests/Node/BinaryNodeTest.php b/src/Symfony/Component/ExpressionLanguage/Tests/Node/BinaryNodeTest.php
@@ -224,6 +224,27 @@ public function testCompileMatchesWithInvalidRegexpAsExpression()
         eval('$regexp = "this is not a regexp"; '.$compiler->getSource().';');
     }
 
+    public function testCompileMatchesWithBooleanBinaryNode()
+    {
+        $binaryNode = new BinaryNode('||', new ConstantNode(true), new ConstantNode(false));
+        $node = new BinaryNode('matches', new ConstantNode('abc'), $binaryNode);
+
+        $this->expectException(SyntaxError::class);
+        $this->expectExceptionMessage('The regex passed to "matches" must be a string');
+        $compiler = new Compiler([]);
+        $node->compile($compiler);
+    }
+
+    public function testCompileMatchesWithStringBinaryNode()
+    {
+        $binaryNode = new BinaryNode('~', new ConstantNode('a'), new ConstantNode('b'));
+        $node = new BinaryNode('matches', new ConstantNode('abc'), $binaryNode);
+
+        $compiler = new Compiler([]);
+        $node->compile($compiler);
+        $this->expectNotToPerformAssertions();
+    }
+
     public function testDivisionByZero()
     {
         $node = new BinaryNode('/', new ConstantNode(1), new ConstantNode(0));

Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,8 @@ public function compile(Compiler $compiler): void`
`52`	`52`	`if ('matches' == $operator) {`
`53`	`53`	`if ($this->nodes['right'] instanceof ConstantNode) {`
`54`	`54`	`$this->evaluateMatches($this->nodes['right']->evaluate([], []), '');`
	`55`	`+ } elseif ($this->nodes['right'] instanceof self && '~' !== $this->nodes['right']->attributes['operator']) {`
	`56`	`+ throw new SyntaxError('The regex passed to "matches" must be a string.');`
`55`	`57`	`}`
`56`	`58`
`57`	`59`	`$compiler`