Skip to content

Commit

Permalink
DB: warn on parameter number constraints
Browse files Browse the repository at this point in the history
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
  • Loading branch information
blizzz committed Jan 8, 2021
1 parent b82a8f7 commit ef28844
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 6 deletions.
30 changes: 30 additions & 0 deletions lib/private/DB/QueryBuilder/QueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,36 @@ public function execute() {
}
}

$numberOfParameters = 0;
$hasTooLargeArrayParameter = false;
foreach ($this->getParameters() as $parameter) {
if (is_array($parameter)) {
$count = count($parameter);
$numberOfParameters += $count;
$hasTooLargeArrayParameter = $hasTooLargeArrayParameter || ($count > 1000);
}
}

if ($hasTooLargeArrayParameter) {
$exception = new QueryException('More than 1000 expressions in a list are not allowed on Oracle.');
$this->logger->logException($exception, [
'message' => 'More than 1000 expressions in a list are not allowed on Oracle.',
'query' => $this->getSQL(),
'level' => ILogger::ERROR,
'app' => 'core',
]);
}

if ($numberOfParameters > 65535) {
$exception = new QueryException('The number of parameters must not exceed 65535. Restriction by PostgreSQL.');
$this->logger->logException($exception, [
'message' => 'The number of parameters must not exceed 65535. Restriction by PostgreSQL.',
'query' => $this->getSQL(),
'level' => ILogger::ERROR,
'app' => 'core',
]);
}

$result = $this->queryBuilder->execute();
if (is_int($result)) {
return $result;
Expand Down
88 changes: 82 additions & 6 deletions tests/lib/DB/QueryBuilder/QueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
namespace Test\DB\QueryBuilder;

use Doctrine\DBAL\Query\Expression\CompositeExpression;
use Doctrine\DBAL\Query\QueryException;
use Doctrine\DBAL\Result;
use OC\DB\QueryBuilder\Literal;
use OC\DB\QueryBuilder\Parameter;
use OC\DB\QueryBuilder\QueryBuilder;
Expand Down Expand Up @@ -1261,6 +1263,10 @@ public function testExecuteWithoutLogger() {
->expects($this->once())
->method('execute')
->willReturn(3);
$queryBuilder
->expects($this->any())
->method('getParameters')
->willReturn([]);
$this->logger
->expects($this->never())
->method('debug');
Expand All @@ -1277,14 +1283,14 @@ public function testExecuteWithoutLogger() {
public function testExecuteWithLoggerAndNamedArray() {
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
$queryBuilder
->expects($this->at(0))
->expects($this->any())
->method('getParameters')
->willReturn([
'foo' => 'bar',
'key' => 'value',
]);
$queryBuilder
->expects($this->at(1))
->expects($this->any())
->method('getSQL')
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
$queryBuilder
Expand Down Expand Up @@ -1315,11 +1321,11 @@ public function testExecuteWithLoggerAndNamedArray() {
public function testExecuteWithLoggerAndUnnamedArray() {
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
$queryBuilder
->expects($this->at(0))
->expects($this->any())
->method('getParameters')
->willReturn(['Bar']);
$queryBuilder
->expects($this->at(1))
->expects($this->any())
->method('getSQL')
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
$queryBuilder
Expand Down Expand Up @@ -1350,11 +1356,11 @@ public function testExecuteWithLoggerAndUnnamedArray() {
public function testExecuteWithLoggerAndNoParams() {
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
$queryBuilder
->expects($this->at(0))
->expects($this->any())
->method('getParameters')
->willReturn([]);
$queryBuilder
->expects($this->at(1))
->expects($this->any())
->method('getSQL')
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
$queryBuilder
Expand All @@ -1380,4 +1386,74 @@ public function testExecuteWithLoggerAndNoParams() {
$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
$this->assertEquals(3, $this->queryBuilder->execute());
}

public function testExecuteWithParameterTooLarge() {
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
$p = array_fill(0, 1001, 'foo');
$queryBuilder
->expects($this->any())
->method('getParameters')
->willReturn([$p]);
$queryBuilder
->expects($this->any())
->method('getSQL')
->willReturn('SELECT * FROM FOO WHERE BAR IN (?)');
$queryBuilder
->expects($this->once())
->method('execute')
->willReturn($this->createMock(Result::class));
$this->logger
->expects($this->once())
->method('logException')
->willReturnCallback(function ($e, $parameters) {
$this->assertInstanceOf(QueryException::class, $e);
$this->assertSame(
'More than 1000 expressions in a list are not allowed on Oracle.',
$parameters['message']
);
});
$this->config
->expects($this->once())
->method('getValue')
->with('log_query', false)
->willReturn(false);

$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
$this->queryBuilder->execute();
}

public function testExecuteWithParametersTooMany() {
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
$p = array_fill(0, 999, 'foo');
$queryBuilder
->expects($this->any())
->method('getParameters')
->willReturn(array_fill(0, 66, $p));
$queryBuilder
->expects($this->any())
->method('getSQL')
->willReturn('SELECT * FROM FOO WHERE BAR IN (?) OR BAR IN (?)');
$queryBuilder
->expects($this->once())
->method('execute')
->willReturn($this->createMock(Result::class));
$this->logger
->expects($this->once())
->method('logException')
->willReturnCallback(function ($e, $parameters) {
$this->assertInstanceOf(QueryException::class, $e);
$this->assertSame(
'The number of parameters must not exceed 65535. Restriction by PostgreSQL.',
$parameters['message']
);
});
$this->config
->expects($this->once())
->method('getValue')
->with('log_query', false)
->willReturn(false);

$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
$this->queryBuilder->execute();
}
}

0 comments on commit ef28844

Please sign in to comment.