diff options
author | Arthur Schiwon <blizzz@arthur-schiwon.de> | 2021-01-08 12:46:10 +0100 |
---|---|---|
committer | Arthur Schiwon <blizzz@arthur-schiwon.de> | 2021-01-11 13:43:34 +0100 |
commit | f9484d15cbf9e41212cedd42a39385b8fc81f11b (patch) | |
tree | 54bf33387338e32bb076b48bb3b14731d481ca51 | |
parent | 45e3261ad5961e78ab919f14ff9234da0ba740b5 (diff) | |
download | nextcloud-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.php | 30 | ||||
-rw-r--r-- | tests/lib/DB/QueryBuilder/QueryBuilderTest.php | 88 |
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(); + } } |