Simplify expression operator parsing and support multi-word operators

src/Database/Expression/QueryExpression.php

@@ -699,38 +699,27 @@ class QueryExpression implements ExpressionInterface, Countable
      * generating the placeholders and replacing the values by them, while storing
      * the value elsewhere for future binding.
      *
-     * @param string $field The value from which the actual field and operator will
+     * @param string $condition The value from which the actual field and operator will
      * be extracted.
      * @param mixed $value The value to be bound to a placeholder for the field
      * @return \Cake\Database\ExpressionInterface
      * @throws \InvalidArgumentException If operator is invalid or missing on NULL usage.
      */
-    protected function _parseCondition(string $field, mixed $value): ExpressionInterface|string
+    protected function _parseCondition(string $condition, mixed $value): ExpressionInterface|string
     {
-        $field = trim($field);
+        $expression = trim($condition);
         $operator = '=';
-        $expression = $field;
-
-        $spaces = substr_count($field, ' ');
-        // Handle operators with a space in them like `is not` and `not like`
-        if ($spaces > 1) {
-            $parts = explode(' ', $field);
-            if (preg_match('/(is not|not \w+)$/i', $field)) {
-                $last = array_pop($parts);
-                $second = array_pop($parts);
-                array_push($parts, strtolower("{$second} {$last}"));
-            }
-            $operator = array_pop($parts);
-            $expression = implode(' ', $parts);
-        } elseif ($spaces == 1) {
-            $parts = explode(' ', $field, 2);
-            [$expression, $operator] = $parts;
-            $operator = strtolower(trim($operator));
+        if (
+            strpos($expression, ' ') !== false &&
+            preg_match('/^(.*?)\s+([^)"`\]\']+)$/', $condition, $matches)
+        ) {
+            $expression = $matches[1];
+            $operator = strtoupper($matches[2]);
         }
-        $type = $this->getTypeMap()->type($expression);
 
+        $type = $this->getTypeMap()->type($expression);
         $typeMultiple = (is_string($type) && str_contains($type, '[]'));
-        if (in_array($operator, ['in', 'not in']) || $typeMultiple) {
+        if (in_array($operator, ['IN', 'NOT IN']) || $typeMultiple) {
             $type = $type ?: 'string';
             if (!$typeMultiple) {
                 $type .= '[]';
@@ -744,7 +733,7 @@ class QueryExpression implements ExpressionInterface, Countable
             $value = $value instanceof ExpressionInterface ? $value : (array)$value;
         }
 
-        if ($operator === 'is' && $value === null) {
+        if ($operator === 'IS' && $value === null) {
             return new UnaryExpression(
                 'IS NULL',
                 new IdentifierExpression($expression),
@@ -752,7 +741,7 @@ class QueryExpression implements ExpressionInterface, Countable
             );
         }
 
-        if ($operator === 'is not' && $value === null) {
+        if ($operator === 'IS NOT' && $value === null) {
             return new UnaryExpression(
                 'IS NOT NULL',
                 new IdentifierExpression($expression),
@@ -760,11 +749,11 @@ class QueryExpression implements ExpressionInterface, Countable
             );
         }
 
-        if ($operator === 'is' && $value !== null) {
+        if ($operator === 'IS' && $value !== null) {
             $operator = '=';
         }
 
-        if ($operator === 'is not' && $value !== null) {
+        if ($operator === 'IS NOT' && $value !== null) {
             $operator = '!=';
         }
 

tests/TestCase/Database/Expression/CaseStatementExpressionTest.php

@@ -801,35 +801,6 @@ class CaseStatementExpressionTest extends TestCase
         );
     }
 
-    public function testSqlInjectionViaUntypedWhenArrayValueIsPossible(): void
-    {
-        $expression = (new CaseStatementExpression())
-            ->when(['1 THEN 1 END; DELETE * FROM foo; --' => '123'])
-            ->then(1);
-
-        $valueBinder = new ValueBinder();
-        $sql = $expression->sql($valueBinder);
-        $this->assertSame(
-            'CASE WHEN 1 THEN 1 END; DELETE * FROM foo; -- :c0 THEN :c1 ELSE NULL END',
-            $sql
-        );
-        $this->assertSame(
-            [
-                ':c0' => [
-                    'value' => '123',
-                    'type' => null,
-                    'placeholder' => 'c0',
-                ],
-                ':c1' => [
-                    'value' => 1,
-                    'type' => 'integer',
-                    'placeholder' => 'c1',
-                ],
-            ],
-            $valueBinder->bindings()
-        );
-    }
-
     public function testSqlInjectionViaTypedThenValueIsNotPossible(): void
     {
         $expression = (new CaseStatementExpression(1))

tests/TestCase/Database/Expression/QueryExpressionTest.php

@@ -41,6 +41,58 @@ class QueryExpressionTest extends TestCase
     }
 
     /**
+     * Tests conditions with multi-word operators.
+     *
+     * @return void
+     */
+    public function testMultiWordOperators(): void
+    {
+        $expr = new QueryExpression(['FUNC(Users.first + Users.last) is not' => 'me']);
+        $this->assertSame('FUNC(Users.first + Users.last) != :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['FUNC(Users.name + Users.id) not similar to' => 'pattern']);
+        $this->assertSame('FUNC(Users.name + Users.id) NOT SIMILAR TO :c0', $expr->sql(new ValueBinder()));
+    }
+
+    /**
+     * Tests conditions with symbol operators.
+     *
+     * @return void
+     */
+    public function testSymbolOperators(): void
+    {
+        $expr = new QueryExpression(['Users.name =' => 'pattern']);
+        $this->assertSame('Users.name = :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name !=' => 'pattern']);
+        $this->assertSame('Users.name != :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name <>' => 'pattern']);
+        $this->assertSame('Users.name <> :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name >=' => 'pattern']);
+        $this->assertSame('Users.name >= :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name <=' => 'pattern']);
+        $this->assertSame('Users.name <= :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name !~' => 'pattern']);
+        $this->assertSame('Users.name !~ :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name *' => 'pattern']);
+        $this->assertSame('Users.name * :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name -' => 'pattern']);
+        $this->assertSame('Users.name - :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name \\' => 'pattern']);
+        $this->assertSame('Users.name \\ :c0', $expr->sql(new ValueBinder()));
+
+        $expr = new QueryExpression(['Users.name @>' => 'pattern']);
+        $this->assertSame('Users.name @> :c0', $expr->sql(new ValueBinder()));
+    }
+
+    /**
      * Test and() and or() calls work transparently
      */
     public function testAndOrCalls(): void

tests/TestCase/Database/QueryTest.php

@@ -1561,7 +1561,7 @@ class QueryTest extends TestCase
 
         $sql = $query->sql();
         $this->assertQuotedQuery(
-            'SELECT <id> FROM <articles> WHERE <id> in \\(:c0,:c1\\)',
+            'SELECT <id> FROM <articles> WHERE <id> IN \\(:c0,:c1\\)',
             $sql,
             !$this->autoQuote
         );
@@ -1602,7 +1602,7 @@ class QueryTest extends TestCase
             ->whereNotInList('id', [1, 3]);
 
         $this->assertQuotedQuery(
-            'SELECT <id> FROM <articles> WHERE <id> not in \\(:c0,:c1\\)',
+            'SELECT <id> FROM <articles> WHERE <id> NOT IN \\(:c0,:c1\\)',
             $query->sql(),
             !$this->autoQuote
         );
@@ -1643,7 +1643,7 @@ class QueryTest extends TestCase
             ->whereNotInListOrNull('id', [1, 3]);
 
         $this->assertQuotedQuery(
-            'SELECT <id> FROM <articles> WHERE \\(<id> not in \\(:c0,:c1\\) OR \\(<id>\\) IS NULL\\)',
+            'SELECT <id> FROM <articles> WHERE \\(<id> NOT IN \\(:c0,:c1\\) OR \\(<id>\\) IS NULL\\)',
             $query->sql(),
             !$this->autoQuote
         );
@@ -2791,7 +2791,7 @@ class QueryTest extends TestCase
         $this->assertQuotedQuery(
             'DELETE FROM <authors> WHERE \(' .
                 '<id> = :c0 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
-                    '<name> not in \(:c1,:c2\) AND \(' .
+                    '<name> NOT IN \(:c1,:c2\) AND \(' .
                         '<name> = :c3 OR <name> = :c4 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c5\)' .
                     '\)' .
                 '\)' .
@@ -3026,7 +3026,7 @@ class QueryTest extends TestCase
         $this->assertQuotedQuery(
             'UPDATE <authors> SET <name> = :c0 WHERE \(' .
                 '<id> = :c1 OR \(<name>\) IS NULL OR \(<email>\) IS NOT NULL OR \(' .
-                    '<name> not in \(:c2,:c3\) AND \(' .
+                    '<name> NOT IN \(:c2,:c3\) AND \(' .
                         '<name> = :c4 OR <name> = :c5 OR \(SELECT <e>\.<name> WHERE <e>\.<name> = :c6\)' .
                     '\)' .
                 '\)' .