[WIP][BUGFIX] Introduce exception for using offset without limit 73/48473/4
authorIan SEBBAGH <ianouf@gmail.com>
Mon, 6 Jun 2016 15:42:26 +0000 (17:42 +0200)
committerJan Helke <typo3@helke.de>
Tue, 7 Jun 2016 12:08:03 +0000 (14:08 +0200)
In Typo3DbBackend::createQueryCommandParametersFromStatementParts,
if an offset is defined, but no limit is, an LogicException is thrown.

SQL does not provide possibility to set offset without limit.
To retrieve all rows from a certain offset up to the end of the
result set, you can use some large number for the limit.
See: http://dev.mysql.com/doc/refman/5.7/en/select.html

Resolves: #65789
Releases: master, 7.6
Change-Id: Icf4db2fbe8dfac21e5da2e32fe5dada38ffd3a77
Reviewed-on: https://review.typo3.org/48473
Reviewed-by: Jan Helke <typo3@helke.de>
Tested-by: Jan Helke <typo3@helke.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/extbase/Classes/Persistence/Generic/Storage/Typo3DbBackend.php
typo3/sysext/extbase/Tests/Functional/Persistence/CountTest.php
typo3/sysext/extbase/Tests/Unit/Persistence/Generic/Storage/Typo3DbBackendTest.php

index 7e18a80..5553d01 100755 (executable)
@@ -358,10 +358,17 @@ class Typo3DbBackend implements BackendInterface, \TYPO3\CMS\Core\SingletonInter
      * that came from a parsed query.
      *
      * @param array $statementParts
+     * @throws \InvalidArgumentException
      * @return array
      */
     protected function createQueryCommandParametersFromStatementParts(array $statementParts)
     {
+        if (isset($statementParts['offset']) && !isset($statementParts['limit'])) {
+            throw new \InvalidArgumentException(
+                'Trying to make query with offset and no limit, the offset would become a limit. You have to set a limit to use offset. To retrieve all rows from a certain offset up to the end of the result set, you can use some large number for the limit.',
+                1465223252
+            );
+        }
         return array(
             'selectFields' => implode(' ', $statementParts['keywords']) . ' ' . implode(',', $statementParts['fields']),
             'fromTable'    => implode(' ', $statementParts['tables']) . ' ' . implode(' ', $statementParts['unions']),
index cad2478..a422ebf 100644 (file)
@@ -90,6 +90,7 @@ class CountTest extends \TYPO3\CMS\Core\Tests\FunctionalTestCase
     {
         $query = $this->postRepository->createQuery();
 
+        $query->setLimit($this->numberOfRecordsInFixture+1);
         $query->setOffset(6);
 
         $this->assertSame(($this->numberOfRecordsInFixture - 6), $query->count());
@@ -102,6 +103,7 @@ class CountTest extends \TYPO3\CMS\Core\Tests\FunctionalTestCase
     {
         $query = $this->postRepository->createQuery();
 
+        $query->setLimit($this->numberOfRecordsInFixture+1);
         $query->setOffset(($this->numberOfRecordsInFixture + 5));
 
         $this->assertSame(0, $query->count());
index 197d827..f150fd7 100644 (file)
@@ -15,6 +15,10 @@ namespace TYPO3\CMS\Extbase\Tests\Unit\Persistence\Generic\Storage;
  */
 
 use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper;
+use TYPO3\CMS\Extbase\Persistence\Generic\QuerySettingsInterface;
+use TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbBackend;
+use TYPO3\CMS\Extbase\Persistence\Generic\Storage\Typo3DbQueryParser;
+use TYPO3\CMS\Extbase\Persistence\QueryInterface;
 
 /**
  * Test case
@@ -127,4 +131,29 @@ class Typo3DbBackendTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
         $result = $mock->_call('resolveParameterPlaceholders', $stmtParts, $parameters);
         $this->assertSame($expected, $result['where']);
     }
+
+    /**
+     * @test
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionCode 1465223252
+     */
+    public function getObjectCountByQueryThrowsExceptionIfOffsetWithoutLimitIsUsed()
+    {
+        $querySettingsProphecy = $this->prophesize(QuerySettingsInterface::class);
+        $queryInterfaceProphecy = $this->prophesize(QueryInterface::class);
+        $queryParserProphecy = $this->prophesize(Typo3DbQueryParser::class);
+        $queryParserProphecy->preparseQuery($queryInterfaceProphecy->reveal())->willReturn([123, []]);
+        $queryParserProphecy->parseQuery($queryInterfaceProphecy->reveal())->willReturn(
+            ['tables' => ['tt_content']]
+        );
+        $queryParserProphecy->addDynamicQueryParts(\Prophecy\Argument::cetera())->willReturn();
+        $queryInterfaceProphecy->getQuerySettings()->willReturn($querySettingsProphecy->reveal());
+        $queryInterfaceProphecy->getConstraint()->willReturn();
+        $queryInterfaceProphecy->getLimit()->willReturn();
+        $queryInterfaceProphecy->getOffset()->willReturn(10);
+
+        $typo3DbQueryParser = new Typo3DbBackend();
+        $typo3DbQueryParser->injectQueryParser($queryParserProphecy->reveal());
+        $typo3DbQueryParser->getObjectCountByQuery($queryInterfaceProphecy->reveal());
+    }
 }