Skip to content

Commit 1c5ad3a

Browse files
committed
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.
1 parent 1125ef2 commit 1c5ad3a

File tree

2 files changed

+62
-2
lines changed

2 files changed

+62
-2
lines changed

src/Sql/Predicate/Predicate.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ public function notLike($identifier, $notLike)
250250
public function expression($expression, $parameters = null)
251251
{
252252
$this->addPredicate(
253-
new Expression($expression, $parameters),
253+
new Expression($expression, func_num_args() > 1 ? $parameters : []),
254254
$this->nextPredicateCombineOperator ?: $this->defaultCombination
255255
);
256256
$this->nextPredicateCombineOperator = null;

test/unit/Sql/Predicate/PredicateTest.php

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@
22

33
namespace LaminasTest\Db\Sql\Predicate;
44

5+
use Laminas\Db\Adapter\Platform\Sql92;
56
use Laminas\Db\Sql\Expression;
67
use Laminas\Db\Sql\Predicate\Predicate;
8+
use Laminas\Db\Sql\Select;
9+
use Laminas\Stdlib\ErrorHandler;
710
use PHPUnit\Framework\TestCase;
811

12+
use const E_USER_NOTICE;
13+
914
class PredicateTest extends TestCase
1015
{
1116
public function testEqualToCreatesOperatorPredicate()
@@ -240,7 +245,7 @@ public function testExpressionNullParameters()
240245
$predicate->expression('foo = bar');
241246
$predicates = $predicate->getPredicates();
242247
$expression = $predicates[0][1];
243-
self::assertEquals([null], $expression->getParameters());
248+
self::assertEquals([], $expression->getParameters());
244249
}
245250

246251
/**
@@ -276,4 +281,59 @@ public function testLiteral()
276281
$predicate->getExpressionData()
277282
);
278283
}
284+
285+
public function testCanCreateExpressionsWithoutAnyBoundSqlParameters(): void
286+
{
287+
$where1 = new Predicate();
288+
289+
$where1->expression('some_expression()');
290+
291+
self::assertSame(
292+
'SELECT "a_table".* FROM "a_table" WHERE (some_expression())',
293+
$this->makeSqlString($where1)
294+
);
295+
}
296+
297+
public function testWillBindSqlParametersToExpressionsWithGivenParameter(): void
298+
{
299+
$where = new Predicate();
300+
301+
$where->expression('some_expression(?)', null);
302+
303+
self::assertSame(
304+
'SELECT "a_table".* FROM "a_table" WHERE (some_expression(\'\'))',
305+
$this->makeSqlString($where)
306+
);
307+
}
308+
309+
public function testWillBindSqlParametersToExpressionsWithGivenStringParameter(): void
310+
{
311+
$where = new Predicate();
312+
313+
$where->expression('some_expression(?)', 'a string');
314+
315+
self::assertSame(
316+
'SELECT "a_table".* FROM "a_table" WHERE (some_expression(\'a string\'))',
317+
$this->makeSqlString($where)
318+
);
319+
}
320+
321+
private function makeSqlString(Predicate $where): string
322+
{
323+
$select = new Select('a_table');
324+
325+
$select->where($where);
326+
327+
// this is still faster than connecting to a real DB for this kind of test.
328+
// we are using unsafe SQL quoting on purpose here: this raises warnings in production.
329+
ErrorHandler::start(E_USER_NOTICE);
330+
331+
try {
332+
$string = $select->getSqlString(new Sql92());
333+
} finally {
334+
ErrorHandler::stop();
335+
}
336+
337+
return $string;
338+
}
279339
}

0 commit comments

Comments
 (0)