Skip to content

Commit

Permalink
[BUGFIX] Prematurely end data array processing on invalid item
Browse files Browse the repository at this point in the history
In case the amount of items, shown in the workspace module,
does not exceed the pagination limit - default 30 - the items
array was previously filled with invalid items till the limit
was reached.

This is now fixed by a guard clause which prematurely ends the
processing (the corresponding loop) as soon as no more item
exists.

This guard clause is also implemented in the calculation of
the start value. Usually there shouldn't be any invalid items,
but as such behaviour can not always be ruled out, especially
on multi-user systems, it seems reasonable to add it here, too.

Resolves: #93915
Relates: #93645
Releases: master, 10.4
Change-Id: I619c9063f12c3b1d5c446f9ca1a0e35e646f90f7
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/68779
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Nikita Hovratov <nikita.h@live.de>
Tested-by: Jochen <rothjochen@gmail.com>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Nikita Hovratov <nikita.h@live.de>
Reviewed-by: Jochen <rothjochen@gmail.com>
Reviewed-by: Benni Mack <benni@typo3.org>
  • Loading branch information
nhovratov authored and bmack committed Apr 16, 2021
1 parent 881906e commit 7258b52
Showing 1 changed file with 21 additions and 8 deletions.
29 changes: 21 additions & 8 deletions typo3/sysext/workspaces/Classes/Service/GridDataService.php
Expand Up @@ -118,15 +118,14 @@ public function generateGridListFromVersions($versions, $parameter, $currentWork
} else {
throw new \InvalidArgumentException('No such workspace defined', 1476048304);
}
$data = [];
$data['data'] = [];
$this->generateDataArray($versions, $filterTxt);
// Only count parent records for pagination
$data['total'] = count(array_filter($this->dataArray, static function ($element) {
return (int)($element[self::GridColumn_CollectionLevel] ?? 0) === 0;
}));
$data['data'] = $this->getDataArray($start, $limit);
return $data;
return [
// Only count parent records for pagination
'total' => count(array_filter($this->dataArray, static function ($element) {
return (int)($element[self::GridColumn_CollectionLevel] ?? 0) === 0;
})),
'data' => $this->getDataArray($start, $limit)
];
}

/**
Expand Down Expand Up @@ -670,6 +669,15 @@ protected function calculateStartWithCollections(int $start): int
// parentRecordsCount only takes the parent records into account
$recordsCount = $parentRecordsCount = 0;
while ($parentRecordsCount < $start) {
// As soon as no more item exists in the dataArray, the loop needs to end
// prematurely to prevent invalid items which would may led to some unexpected
// behaviour. Note: This usually should never happen since these records must
// exists => As they were responsible for increasing the start value. However to
// prevent errors in case multiple different users manipulate the records count
// somehow simultaneously, we apply this check to be save.
if (!isset($this->dataArray[$recordsCount])) {
break;
}
// Loop over the dataArray until we found enough parent records
$item = $this->dataArray[$recordsCount];
if (($item[self::GridColumn_CollectionLevel] ?? 0) === 0) {
Expand Down Expand Up @@ -706,6 +714,11 @@ private function fillDataArrayPart(int $start, int $end): array
// parentRecordsCount only takes the parent records into account.
$itemsCount = $parentRecordsCount = $start;
while ($parentRecordsCount < $end) {
// As soon as no more item exists in the dataArray, the loop needs to end
// prematurely to prevent invalid items which would trigger JavaScript errors.
if (!isset($this->dataArray[$itemsCount])) {
break;
}
// Loop over the dataArray until we found enough parent records
$item = $this->dataArray[$itemsCount];
// Add the item to the $dataArrayPart
Expand Down

0 comments on commit 7258b52

Please sign in to comment.