summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorArthur Schiwon <blizzz@arthur-schiwon.de>2021-01-08 12:46:10 +0100
committerArthur Schiwon <blizzz@arthur-schiwon.de>2021-01-11 13:43:34 +0100
commitf9484d15cbf9e41212cedd42a39385b8fc81f11b (patch)
tree54bf33387338e32bb076b48bb3b14731d481ca51
parent45e3261ad5961e78ab919f14ff9234da0ba740b5 (diff)
downloadnextcloud-server-f9484d15cbf9e41212cedd42a39385b8fc81f11b.tar.gz
nextcloud-server-f9484d15cbf9e41212cedd42a39385b8fc81f11b.zip
DB: warn on parameter number constraints
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
-rw-r--r--lib/private/DB/QueryBuilder/QueryBuilder.php30
-rw-r--r--tests/lib/DB/QueryBuilder/QueryBuilderTest.php88
2 files changed, 112 insertions, 6 deletions
diff --git a/lib/private/DB/QueryBuilder/QueryBuilder.php b/lib/private/DB/QueryBuilder/QueryBuilder.php
index 657e52e54bc..fb28fa28649 100644
--- a/lib/private/DB/QueryBuilder/QueryBuilder.php
+++ b/lib/private/DB/QueryBuilder/QueryBuilder.php
@@ -253,6 +253,36 @@ class QueryBuilder implements IQueryBuilder {
}
}
+ $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;
diff --git a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php
index fe87a201d57..aef1acc40c1 100644
--- a/tests/lib/DB/QueryBuilder/QueryBuilderTest.php
+++ b/tests/lib/DB/QueryBuilder/QueryBuilderTest.php
@@ -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;
@@ -1261,6 +1263,10 @@ class QueryBuilderTest extends \Test\TestCase {
->expects($this->once())
->method('execute')
->willReturn(3);
+ $queryBuilder
+ ->expects($this->any())
+ ->method('getParameters')
+ ->willReturn([]);
$this->logger
->expects($this->never())
->method('debug');
@@ -1277,14 +1283,14 @@ class QueryBuilderTest extends \Test\TestCase {
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
@@ -1315,11 +1321,11 @@ class QueryBuilderTest extends \Test\TestCase {
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
@@ -1350,11 +1356,11 @@ class QueryBuilderTest extends \Test\TestCase {
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
@@ -1380,4 +1386,74 @@ class QueryBuilderTest extends \Test\TestCase {
$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();
+ }
}