From 1c5ad3a31a6ec7880a12a352ae1739322fc4a32b Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Mon, 5 Dec 2022 19:09:13 +0100
Subject: [PATCH] Fix: prevent bound parameters being `[null]` when no
 parameters are given to `Predicate#expression()`

Around `laminas/laminas-db:2.10.1`, a regression was introduced, in which calling
`Laminas\Db\Sql\Predicate#expression("an_expression()")` led to crashes like following
in downstream consumers:

```
Laminas\Db\Sql\Exception\RuntimeException: The number of replacements in the expression does not match the number of parameters

vendor/laminas/laminas-db/src/Sql/Expression.php:151
vendor/laminas/laminas-db/src/Sql/Predicate/PredicateSet.php:178
vendor/laminas/laminas-db/src/Sql/Predicate/PredicateSet.php:178
vendor/laminas/laminas-db/src/Sql/Predicate/PredicateSet.php:178
vendor/laminas/laminas-db/src/Sql/AbstractSql.php:129
vendor/laminas/laminas-db/src/Sql/Select.php:633
```

This was because predicates were initialized with an `array{null}` by
default, when expressions like `$sql->where->expression("some_expression()")`
were used.

The usage of `$sql->where->expression("some_expression()", "foo")` remains
unchanged with this patch.

This fix targets `2.15.x`, and attempts to make predicates safe to use when no
parameters have been given.

While an existing test has indeed been changed, this shouldn't have any effect for
downstream consumers, since `Predicate#expression(string)` didn't work (so far)
anyway, due to the kind of crash highlighted above.
---
 src/Sql/Predicate/Predicate.php           |  2 +-
 test/unit/Sql/Predicate/PredicateTest.php | 62 ++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/Sql/Predicate/Predicate.php b/src/Sql/Predicate/Predicate.php
index a18d6b43..41a00006 100644
--- a/src/Sql/Predicate/Predicate.php
+++ b/src/Sql/Predicate/Predicate.php
@@ -250,7 +250,7 @@ public function notLike($identifier, $notLike)
     public function expression($expression, $parameters = null)
     {
         $this->addPredicate(
-            new Expression($expression, $parameters),
+            new Expression($expression, func_num_args() > 1 ? $parameters : []),
             $this->nextPredicateCombineOperator ?: $this->defaultCombination
         );
         $this->nextPredicateCombineOperator = null;
diff --git a/test/unit/Sql/Predicate/PredicateTest.php b/test/unit/Sql/Predicate/PredicateTest.php
index 4b7ec5f6..c8489136 100644
--- a/test/unit/Sql/Predicate/PredicateTest.php
+++ b/test/unit/Sql/Predicate/PredicateTest.php
@@ -2,10 +2,15 @@
 
 namespace LaminasTest\Db\Sql\Predicate;
 
+use Laminas\Db\Adapter\Platform\Sql92;
 use Laminas\Db\Sql\Expression;
 use Laminas\Db\Sql\Predicate\Predicate;
+use Laminas\Db\Sql\Select;
+use Laminas\Stdlib\ErrorHandler;
 use PHPUnit\Framework\TestCase;
 
+use const E_USER_NOTICE;
+
 class PredicateTest extends TestCase
 {
     public function testEqualToCreatesOperatorPredicate()
@@ -240,7 +245,7 @@ public function testExpressionNullParameters()
         $predicate->expression('foo = bar');
         $predicates = $predicate->getPredicates();
         $expression = $predicates[0][1];
-        self::assertEquals([null], $expression->getParameters());
+        self::assertEquals([], $expression->getParameters());
     }
 
     /**
@@ -276,4 +281,59 @@ public function testLiteral()
             $predicate->getExpressionData()
         );
     }
+
+    public function testCanCreateExpressionsWithoutAnyBoundSqlParameters(): void
+    {
+        $where1 = new Predicate();
+
+        $where1->expression('some_expression()');
+
+        self::assertSame(
+            'SELECT "a_table".* FROM "a_table" WHERE (some_expression())',
+            $this->makeSqlString($where1)
+        );
+    }
+
+    public function testWillBindSqlParametersToExpressionsWithGivenParameter(): void
+    {
+        $where = new Predicate();
+
+        $where->expression('some_expression(?)', null);
+
+        self::assertSame(
+            'SELECT "a_table".* FROM "a_table" WHERE (some_expression(\'\'))',
+            $this->makeSqlString($where)
+        );
+    }
+
+    public function testWillBindSqlParametersToExpressionsWithGivenStringParameter(): void
+    {
+        $where = new Predicate();
+
+        $where->expression('some_expression(?)', 'a string');
+
+        self::assertSame(
+            'SELECT "a_table".* FROM "a_table" WHERE (some_expression(\'a string\'))',
+            $this->makeSqlString($where)
+        );
+    }
+
+    private function makeSqlString(Predicate $where): string
+    {
+        $select = new Select('a_table');
+
+        $select->where($where);
+
+        // this is still faster than connecting to a real DB for this kind of test.
+        // we are using unsafe SQL quoting on purpose here: this raises warnings in production.
+        ErrorHandler::start(E_USER_NOTICE);
+
+        try {
+            $string = $select->getSqlString(new Sql92());
+        } finally {
+            ErrorHandler::stop();
+        }
+
+        return $string;
+    }
 }