Merge pull request #25121 from nextcloud/backport/25036/stable19
[stable19] respect DB restrictions on number of arguments in statements and queries
This commit is contained in:
commit
a4c7a62394
|
@ -27,6 +27,7 @@
|
||||||
namespace OCA\User_LDAP\Mapping;
|
namespace OCA\User_LDAP\Mapping;
|
||||||
|
|
||||||
use OC\DB\QueryBuilder\QueryBuilder;
|
use OC\DB\QueryBuilder\QueryBuilder;
|
||||||
|
use OCP\DB\QueryBuilder\IQueryBuilder;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Class AbstractMapping
|
* Class AbstractMapping
|
||||||
|
@ -188,19 +189,59 @@ abstract class AbstractMapping {
|
||||||
return $this->cache[$fdn];
|
return $this->cache[$fdn];
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getListOfIdsByDn(array $fdns): array {
|
protected function prepareListOfIdsQuery(array $dnList): IQueryBuilder {
|
||||||
$qb = $this->dbc->getQueryBuilder();
|
$qb = $this->dbc->getQueryBuilder();
|
||||||
$qb->select('owncloud_name', 'ldap_dn')
|
$qb->select('owncloud_name', 'ldap_dn')
|
||||||
->from($this->getTableName(false))
|
->from($this->getTableName(false))
|
||||||
->where($qb->expr()->in('ldap_dn', $qb->createNamedParameter($fdns, QueryBuilder::PARAM_STR_ARRAY)));
|
->where($qb->expr()->in('ldap_dn', $qb->createNamedParameter($dnList, QueryBuilder::PARAM_STR_ARRAY)));
|
||||||
$stmt = $qb->execute();
|
return $qb;
|
||||||
|
}
|
||||||
|
|
||||||
$results = $stmt->fetchAll(\Doctrine\DBAL\FetchMode::ASSOCIATIVE);
|
protected function collectResultsFromListOfIdsQuery(IQueryBuilder $qb, array &$results): void {
|
||||||
foreach ($results as $key => $entry) {
|
$stmt = $qb->execute();
|
||||||
unset($results[$key]);
|
while ($entry = $stmt->fetch(\Doctrine\DBAL\FetchMode::ASSOCIATIVE)) {
|
||||||
$results[$entry['ldap_dn']] = $entry['owncloud_name'];
|
$results[$entry['ldap_dn']] = $entry['owncloud_name'];
|
||||||
$this->cache[$entry['ldap_dn']] = $entry['owncloud_name'];
|
$this->cache[$entry['ldap_dn']] = $entry['owncloud_name'];
|
||||||
}
|
}
|
||||||
|
$stmt->closeCursor();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getListOfIdsByDn(array $fdns): array {
|
||||||
|
$totalDBParamLimit = 65000;
|
||||||
|
$sliceSize = 1000;
|
||||||
|
$maxSlices = $totalDBParamLimit / $sliceSize;
|
||||||
|
$results = [];
|
||||||
|
|
||||||
|
$slice = 1;
|
||||||
|
$fdnsSlice = count($fdns) > $sliceSize ? array_slice($fdns, 0, $sliceSize) : $fdns;
|
||||||
|
$qb = $this->prepareListOfIdsQuery($fdnsSlice);
|
||||||
|
|
||||||
|
while (isset($fdnsSlice[999])) {
|
||||||
|
// Oracle does not allow more than 1000 values in the IN list,
|
||||||
|
// but allows slicing
|
||||||
|
$slice++;
|
||||||
|
$fdnsSlice = array_slice($fdns, $sliceSize * ($slice - 1), $sliceSize);
|
||||||
|
|
||||||
|
/** @see https://github.com/vimeo/psalm/issues/4995 */
|
||||||
|
/** @psalm-suppress TypeDoesNotContainType */
|
||||||
|
if (!isset($qb)) {
|
||||||
|
$qb = $this->prepareListOfIdsQuery($fdnsSlice);
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!empty($fdnsSlice)) {
|
||||||
|
$qb->orWhere($qb->expr()->in('ldap_dn', $qb->createNamedParameter($fdnsSlice, QueryBuilder::PARAM_STR_ARRAY)));
|
||||||
|
}
|
||||||
|
|
||||||
|
if ($slice % $maxSlices === 0) {
|
||||||
|
$this->collectResultsFromListOfIdsQuery($qb, $results);
|
||||||
|
unset($qb);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (isset($qb)) {
|
||||||
|
$this->collectResultsFromListOfIdsQuery($qb, $results);
|
||||||
|
}
|
||||||
|
|
||||||
return $results;
|
return $results;
|
||||||
}
|
}
|
||||||
|
|
|
@ -281,4 +281,23 @@ abstract class AbstractMappingTest extends \Test\TestCase {
|
||||||
$results = $mapper->getList(1, 1);
|
$results = $mapper->getList(1, 1);
|
||||||
$this->assertSame(1, count($results));
|
$this->assertSame(1, count($results));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testGetListOfIdsByDn() {
|
||||||
|
/** @var AbstractMapping $mapper */
|
||||||
|
list($mapper,) = $this->initTest();
|
||||||
|
|
||||||
|
$listOfDNs = [];
|
||||||
|
for ($i = 0; $i < 66640; $i++) {
|
||||||
|
// Postgres has a limit of 65535 values in a single IN list
|
||||||
|
$name = 'as_' . $i;
|
||||||
|
$dn = 'uid=' . $name . ',dc=example,dc=org';
|
||||||
|
$listOfDNs[] = $dn;
|
||||||
|
if ($i % 20 === 0) {
|
||||||
|
$mapper->map($dn, $name, 'fake-uuid-' . $i);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
$result = $mapper->getListOfIdsByDn($listOfDNs);
|
||||||
|
$this->assertSame(66640 / 20, count($result));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -214,6 +214,35 @@ class QueryBuilder implements IQueryBuilder {
|
||||||
}
|
}
|
||||||
|
|
||||||
return $this->queryBuilder->execute();
|
return $this->queryBuilder->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',
|
||||||
|
]);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -22,6 +22,8 @@
|
||||||
namespace Test\DB\QueryBuilder;
|
namespace Test\DB\QueryBuilder;
|
||||||
|
|
||||||
use Doctrine\DBAL\Query\Expression\CompositeExpression;
|
use Doctrine\DBAL\Query\Expression\CompositeExpression;
|
||||||
|
use Doctrine\DBAL\Query\QueryException;
|
||||||
|
use Doctrine\DBAL\Result;
|
||||||
use OC\DB\QueryBuilder\Literal;
|
use OC\DB\QueryBuilder\Literal;
|
||||||
use OC\DB\QueryBuilder\Parameter;
|
use OC\DB\QueryBuilder\Parameter;
|
||||||
use OC\DB\QueryBuilder\QueryBuilder;
|
use OC\DB\QueryBuilder\QueryBuilder;
|
||||||
|
@ -1223,6 +1225,10 @@ class QueryBuilderTest extends \Test\TestCase {
|
||||||
->expects($this->once())
|
->expects($this->once())
|
||||||
->method('execute')
|
->method('execute')
|
||||||
->willReturn(3);
|
->willReturn(3);
|
||||||
|
$queryBuilder
|
||||||
|
->expects($this->any())
|
||||||
|
->method('getParameters')
|
||||||
|
->willReturn([]);
|
||||||
$this->logger
|
$this->logger
|
||||||
->expects($this->never())
|
->expects($this->never())
|
||||||
->method('debug');
|
->method('debug');
|
||||||
|
@ -1239,14 +1245,14 @@ class QueryBuilderTest extends \Test\TestCase {
|
||||||
public function testExecuteWithLoggerAndNamedArray() {
|
public function testExecuteWithLoggerAndNamedArray() {
|
||||||
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
|
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
->expects($this->at(0))
|
->expects($this->any())
|
||||||
->method('getParameters')
|
->method('getParameters')
|
||||||
->willReturn([
|
->willReturn([
|
||||||
'foo' => 'bar',
|
'foo' => 'bar',
|
||||||
'key' => 'value',
|
'key' => 'value',
|
||||||
]);
|
]);
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
->expects($this->at(1))
|
->expects($this->any())
|
||||||
->method('getSQL')
|
->method('getSQL')
|
||||||
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
|
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
|
@ -1277,11 +1283,11 @@ class QueryBuilderTest extends \Test\TestCase {
|
||||||
public function testExecuteWithLoggerAndUnnamedArray() {
|
public function testExecuteWithLoggerAndUnnamedArray() {
|
||||||
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
|
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
->expects($this->at(0))
|
->expects($this->any())
|
||||||
->method('getParameters')
|
->method('getParameters')
|
||||||
->willReturn(['Bar']);
|
->willReturn(['Bar']);
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
->expects($this->at(1))
|
->expects($this->any())
|
||||||
->method('getSQL')
|
->method('getSQL')
|
||||||
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
|
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
|
@ -1312,11 +1318,11 @@ class QueryBuilderTest extends \Test\TestCase {
|
||||||
public function testExecuteWithLoggerAndNoParams() {
|
public function testExecuteWithLoggerAndNoParams() {
|
||||||
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
|
$queryBuilder = $this->createMock(\Doctrine\DBAL\Query\QueryBuilder::class);
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
->expects($this->at(0))
|
->expects($this->any())
|
||||||
->method('getParameters')
|
->method('getParameters')
|
||||||
->willReturn([]);
|
->willReturn([]);
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
->expects($this->at(1))
|
->expects($this->any())
|
||||||
->method('getSQL')
|
->method('getSQL')
|
||||||
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
|
->willReturn('SELECT * FROM FOO WHERE BAR = ?');
|
||||||
$queryBuilder
|
$queryBuilder
|
||||||
|
@ -1342,4 +1348,74 @@ class QueryBuilderTest extends \Test\TestCase {
|
||||||
$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
|
$this->invokePrivate($this->queryBuilder, 'queryBuilder', [$queryBuilder]);
|
||||||
$this->assertEquals(3, $this->queryBuilder->execute());
|
$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();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue