From 08625d368dc2e6534a0f250941eb25598f8e4a95 Mon Sep 17 00:00:00 2001 From: Oliver Hader <oliver@typo3.org> Date: Fri, 16 Sep 2022 09:23:37 +0200 Subject: [PATCH] [TASK] Prevent undefined array key warnings in ext:beuser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change addresses several "undefined array key" issues that have been identified by PsalmPHP (see issue #98321). Resolves: #98347 Related: #98321 Releases: main, 11.5 Change-Id: Ibca701d77324a4b2fe1d4a416deba5758d1ebde2 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/75732 Tested-by: core-ci <typo3@b13.com> Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Oliver Hader <oliver.hader@typo3.org> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de> Reviewed-by: Oliver Hader <oliver.hader@typo3.org> --- .../Controller/BackendUserController.php | 5 +-- .../Controller/PermissionController.php | 9 ++--- .../Classes/Domain/Model/BackendUser.php | 2 +- .../Service/UserInformationService.php | 36 ++++++++++--------- .../ViewHelpers/ArrayElementViewHelper.php | 1 + .../Display/TableAccessViewHelper.php | 9 +++-- .../ViewHelpers/PermissionsViewHelper.php | 34 +++++++++++------- .../SpriteIconForRecordViewHelper.php | 3 ++ .../ViewHelpers/SwitchUserViewHelper.php | 2 ++ 9 files changed, 63 insertions(+), 38 deletions(-) diff --git a/typo3/sysext/beuser/Classes/Controller/BackendUserController.php b/typo3/sysext/beuser/Classes/Controller/BackendUserController.php index 70dbb9e534e3..7626debbcb30 100644 --- a/typo3/sysext/beuser/Classes/Controller/BackendUserController.php +++ b/typo3/sysext/beuser/Classes/Controller/BackendUserController.php @@ -160,7 +160,7 @@ class BackendUserController extends ActionController 'totalAmountOfBackendUsers' => $backendUsers->count(), 'backendUserGroups' => array_merge([''], $this->backendUserGroupRepository->findAll()->toArray()), 'compareUserUidList' => array_combine($compareUserList, $compareUserList), - 'currentUserUid' => $backendUser->user['uid'], + 'currentUserUid' => $backendUser->user['uid'] ?? null, 'compareUserList' => !empty($compareUserList) ? $this->backendUserRepository->findByUidList($compareUserList) : '', ]); @@ -245,10 +245,11 @@ class BackendUserController extends ActionController 'returnUrl' => $this->request->getAttribute('normalizedParams')->getRequestUri(), ])); $buttonBar->addButton($addUserButton); + $username = empty($data['user']['username']) ? '' : ': ' . $data['user']['username']; $shortcutButton = $buttonBar->makeShortcutButton() ->setRouteIdentifier('system_BeuserTxBeuser') ->setArguments(['action' => 'show', 'uid' => $uid]) - ->setDisplayName(LocalizationUtility::translate('backendUser', 'beuser') . ': ' . (string)$data['user']['username']); + ->setDisplayName(LocalizationUtility::translate('backendUser', 'beuser') . $username); $buttonBar->addButton($shortcutButton, ButtonBar::BUTTON_POSITION_RIGHT); return $this->moduleTemplate->renderResponse('BackendUser/Show'); diff --git a/typo3/sysext/beuser/Classes/Controller/PermissionController.php b/typo3/sysext/beuser/Classes/Controller/PermissionController.php index 80ef92204331..2f1b52847bb0 100644 --- a/typo3/sysext/beuser/Classes/Controller/PermissionController.php +++ b/typo3/sysext/beuser/Classes/Controller/PermissionController.php @@ -351,13 +351,14 @@ class PermissionController $dataHandlerInput = []; // Prepare the input data for data handler - if (is_array($data['pages'] ?? false) && $data['pages'] !== []) { - foreach ($data['pages'] as $pageUid => $properties) { + $dataPages = $data['pages'] ?? null; + if (is_array($dataPages) && $dataPages !== []) { + foreach ($dataPages as $pageUid => $properties) { // if the owner and group field shouldn't be touched, unset the option - if ((int)$properties['perms_userid'] === -1) { + if ((int)($properties['perms_userid'] ?? 0) === -1) { unset($properties['perms_userid']); } - if ((int)$properties['perms_groupid'] === -1) { + if ((int)($properties['perms_groupid'] ?? 0) === -1) { unset($properties['perms_groupid']); } $dataHandlerInput[$pageUid] = $properties; diff --git a/typo3/sysext/beuser/Classes/Domain/Model/BackendUser.php b/typo3/sysext/beuser/Classes/Domain/Model/BackendUser.php index b764a1f577be..7d59132f3629 100644 --- a/typo3/sysext/beuser/Classes/Domain/Model/BackendUser.php +++ b/typo3/sysext/beuser/Classes/Domain/Model/BackendUser.php @@ -182,7 +182,7 @@ class BackendUser extends AbstractEntity */ public function isCurrentlyLoggedIn() { - return $this->getUid() === (int)$this->getBackendUser()->user['uid']; + return $this->getUid() === (int)($this->getBackendUser()->user['uid'] ?? 0); } /** diff --git a/typo3/sysext/beuser/Classes/Service/UserInformationService.php b/typo3/sysext/beuser/Classes/Service/UserInformationService.php index c5c8742649cd..384d65a650c7 100644 --- a/typo3/sysext/beuser/Classes/Service/UserInformationService.php +++ b/typo3/sysext/beuser/Classes/Service/UserInformationService.php @@ -114,7 +114,7 @@ class UserInformationService 'user' => $user->user ?? [], 'groups' => [ 'inherit' => $user->userGroupsUID, - 'direct' => GeneralUtility::trimExplode(',', $user->user['usergroup'], true), + 'direct' => GeneralUtility::trimExplode(',', $user->user['usergroup'] ?? '', true), ], 'modules' => [], ]; @@ -122,16 +122,17 @@ class UserInformationService foreach ($data['groups'] as $type => $groups) { foreach ($groups as $key => $id) { $record = BackendUtility::getRecord('be_groups', (int)$id); - if ($record) { - $data['groups']['all'][$record['uid']]['row'] = $record; - $data['groups']['all'][$record['uid']][$type] = 1; + if (isset($record['uid'])) { + $recordId = $record['uid']; + $data['groups']['all'][$recordId]['row'] = $record; + $data['groups']['all'][$recordId][$type] = 1; } } } // languages $siteLanguages = $this->getAllSiteLanguages(); - $userLanguages = GeneralUtility::trimExplode(',', $user->groupData['allowed_languages'], true); + $userLanguages = GeneralUtility::trimExplode(',', $user->groupData['allowed_languages'] ?? '', true); asort($userLanguages); foreach ($userLanguages as $languageId) { $languageId = (int)$languageId; @@ -145,7 +146,7 @@ class UserInformationService $data['tables']['tables_select'] = []; $data['tables']['tables_modify'] = []; foreach (['tables_select', 'tables_modify'] as $tableField) { - $temp = GeneralUtility::trimExplode(',', $user->groupData[$tableField], true); + $temp = GeneralUtility::trimExplode(',', $user->groupData[$tableField] ?? '', true); foreach ($temp as $tableName) { if (isset($GLOBALS['TCA'][$tableName]['ctrl']['title'])) { $data['tables'][$tableField][$tableName] = $GLOBALS['TCA'][$tableName]['ctrl']['title']; @@ -155,7 +156,7 @@ class UserInformationService $data['tables']['all'] = array_replace($data['tables']['tables_select'] ?? [], $data['tables']['tables_modify'] ?? []); // DB mounts - $dbMounts = GeneralUtility::trimExplode(',', $user->groupData['webmounts'], true); + $dbMounts = GeneralUtility::trimExplode(',', $user->groupData['webmounts'] ?? '', true); asort($dbMounts); foreach ($dbMounts as $mount) { $record = BackendUtility::getRecord('pages', (int)$mount); @@ -165,7 +166,7 @@ class UserInformationService } // File mounts - $fileMounts = GeneralUtility::trimExplode(',', $user->groupData['filemounts'], true); + $fileMounts = GeneralUtility::trimExplode(',', $user->groupData['filemounts'] ?? '', true); asort($fileMounts); foreach ($fileMounts as $mount) { $record = BackendUtility::getRecord('sys_filemounts', (int)$mount); @@ -175,7 +176,7 @@ class UserInformationService } // Modules - $modules = GeneralUtility::trimExplode(',', $user->groupData['modules'], true); + $modules = GeneralUtility::trimExplode(',', $user->groupData['modules'] ?? '', true); foreach ($modules as $moduleIdentifier) { if ($this->moduleProvider->isModuleRegistered($moduleIdentifier)) { $data['modules'][] = $this->moduleProvider->getModule($moduleIdentifier); @@ -200,7 +201,8 @@ class UserInformationService } // file & folder permissions - if ($filePermissions = $user->groupData['file_permissions']) { + $filePermissions = $user->groupData['file_permissions'] ?? ''; + if ($filePermissions) { $items = GeneralUtility::trimExplode(',', $filePermissions, true); foreach ($GLOBALS['TCA']['be_groups']['columns']['file_permissions']['config']['items'] as $availableItem) { if (in_array($availableItem[1], $items, true)) { @@ -213,13 +215,15 @@ class UserInformationService $data['tsconfig'] = $user->getTSConfig(); // non_exclude_fields - $fieldListTmp = GeneralUtility::trimExplode(',', $user->groupData['non_exclude_fields'], true); + $fieldListTmp = GeneralUtility::trimExplode(',', $user->groupData['non_exclude_fields'] ?? '', true); $fieldList = []; foreach ($fieldListTmp as $item) { - $split = explode(':', $item); - if (isset($GLOBALS['TCA'][$split[0]]['ctrl']['title'])) { - $fieldList[$split[0]]['label'] = $GLOBALS['TCA'][$split[0]]['ctrl']['title']; - $fieldList[$split[0]]['fields'][$split[1]] = $GLOBALS['TCA'][$split[0]]['columns'][$split[1]]['label'] ?? $split[1]; + $itemParts = explode(':', $item); + $itemTable = $itemParts[0]; + $itemField = $itemParts[1] ?? ''; + if (!empty($itemField) && isset($GLOBALS['TCA'][$itemTable]['ctrl']['title'])) { + $fieldList[$itemTable]['label'] = $GLOBALS['TCA'][$itemTable]['ctrl']['title']; + $fieldList[$itemTable]['fields'][$itemField] = $GLOBALS['TCA'][$itemTable]['columns'][$itemField]['label'] ?? $itemField; } } ksort($fieldList); @@ -232,7 +236,7 @@ class UserInformationService $specialItems = $GLOBALS['TCA']['pages']['columns']['doktype']['config']['items']; foreach ($specialItems as $specialItem) { $value = $specialItem[1]; - if (!GeneralUtility::inList($user->groupData['pagetypes_select'], $value)) { + if (!GeneralUtility::inList($user->groupData['pagetypes_select'] ?? '', $value)) { continue; } $label = $specialItem[0]; diff --git a/typo3/sysext/beuser/Classes/ViewHelpers/ArrayElementViewHelper.php b/typo3/sysext/beuser/Classes/ViewHelpers/ArrayElementViewHelper.php index 7b3864d52a15..5302c76514c1 100644 --- a/typo3/sysext/beuser/Classes/ViewHelpers/ArrayElementViewHelper.php +++ b/typo3/sysext/beuser/Classes/ViewHelpers/ArrayElementViewHelper.php @@ -41,6 +41,7 @@ final class ArrayElementViewHelper extends AbstractViewHelper /** * Return array element by key. Accessed values must be scalar (string, int, float or double) * + * @param array{'array': array, 'key': string, 'subKey': string} $arguments * @throws Exception */ public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext): string diff --git a/typo3/sysext/beuser/Classes/ViewHelpers/Display/TableAccessViewHelper.php b/typo3/sysext/beuser/Classes/ViewHelpers/Display/TableAccessViewHelper.php index 979241a7ff63..5e02358bf85b 100644 --- a/typo3/sysext/beuser/Classes/ViewHelpers/Display/TableAccessViewHelper.php +++ b/typo3/sysext/beuser/Classes/ViewHelpers/Display/TableAccessViewHelper.php @@ -33,9 +33,14 @@ final class TableAccessViewHelper extends AbstractConditionViewHelper $this->registerArgument('modify', 'array', 'List of allowed tables to modify', false, []); } + /** + * @param array{table?: string, select?: array, modify?: array} $arguments + */ public static function verdict(array $arguments, RenderingContextInterface $renderingContext): bool { - $table = $arguments['table']; - return array_key_exists($table, (array)$arguments['select']) || array_key_exists($table, (array)$arguments['modify']); + $table = (string)($arguments['table'] ?? ''); + $select = (array)($arguments['select'] ?? []); + $modify = (array)($arguments['modify'] ?? []); + return array_key_exists($table, $select) || array_key_exists($table, $modify); } } diff --git a/typo3/sysext/beuser/Classes/ViewHelpers/PermissionsViewHelper.php b/typo3/sysext/beuser/Classes/ViewHelpers/PermissionsViewHelper.php index 2fc658537822..5b6e67da0579 100644 --- a/typo3/sysext/beuser/Classes/ViewHelpers/PermissionsViewHelper.php +++ b/typo3/sysext/beuser/Classes/ViewHelpers/PermissionsViewHelper.php @@ -38,6 +38,8 @@ final class PermissionsViewHelper extends AbstractViewHelper { use CompileWithRenderStatic; + protected const MASKS = [1, 16, 2, 4, 8]; + /** * As this ViewHelper renders HTML, the output must not be escaped. * @@ -51,25 +53,21 @@ final class PermissionsViewHelper extends AbstractViewHelper { $this->registerArgument('permission', 'int', 'Current permission', true); $this->registerArgument('scope', 'string', '"user" / "group" / "everybody"', true); - $this->registerArgument('pageId', 'int', '', true); + $this->registerArgument('pageId', 'int', 'Page ID to evaluate permission for', true); } + /** + * @param array{permission: int, scope: string, pageId: int} $arguments + * @param \Closure $renderChildrenClosure + * @param RenderingContextInterface $renderingContext + * @return string + */ public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext): string { $iconFactory = GeneralUtility::makeInstance(IconFactory::class); - $masks = [1, 16, 2, 4, 8]; - - if (empty(self::$cachePermissionLabels)) { - foreach ($masks as $mask) { - self::$cachePermissionLabels[$mask] = htmlspecialchars(self::getLanguageService()->sL( - 'LLL:EXT:beuser/Resources/Private/Language/locallang_mod_permission.xlf:' . $mask, - )); - } - } - $icon = ''; - foreach ($masks as $mask) { + foreach (self::MASKS as $mask) { if ($arguments['permission'] & $mask) { $iconIdentifier = 'actions-check'; $iconClass = 'text-success'; @@ -80,7 +78,7 @@ final class PermissionsViewHelper extends AbstractViewHelper $mode = 'add'; } - $label = self::$cachePermissionLabels[$mask]; + $label = self::resolvePermissionLabel($mask); $icon .= '<button' . ' aria-label="' . htmlspecialchars($label) . ', ' . htmlspecialchars($mode) . ', ' . htmlspecialchars($arguments['scope']) . '"' . ' title="' . htmlspecialchars($label) . '"' @@ -98,6 +96,16 @@ final class PermissionsViewHelper extends AbstractViewHelper return '<span id="' . htmlspecialchars($arguments['pageId'] . '_' . $arguments['scope']) . '">' . $icon . '</span>'; } + protected static function resolvePermissionLabel(int $mask): string + { + if (!isset(self::$cachePermissionLabels[$mask])) { + self::$cachePermissionLabels[$mask] = htmlspecialchars(self::getLanguageService()->sL( + 'LLL:EXT:beuser/Resources/Private/Language/locallang_mod_permission.xlf:' . $mask, + )); + } + return self::$cachePermissionLabels[$mask]; + } + protected static function getLanguageService(): LanguageService { return $GLOBALS['LANG']; diff --git a/typo3/sysext/beuser/Classes/ViewHelpers/SpriteIconForRecordViewHelper.php b/typo3/sysext/beuser/Classes/ViewHelpers/SpriteIconForRecordViewHelper.php index c0ed366b07a9..2d3bb3dcbbb9 100644 --- a/typo3/sysext/beuser/Classes/ViewHelpers/SpriteIconForRecordViewHelper.php +++ b/typo3/sysext/beuser/Classes/ViewHelpers/SpriteIconForRecordViewHelper.php @@ -52,6 +52,9 @@ final class SpriteIconForRecordViewHelper extends AbstractBackendViewHelper return self::renderStatic($this->arguments, $this->buildRenderChildrenClosure(), $this->renderingContext); } + /** + * @param array{'table': string, 'object': object} $arguments + */ public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext): string { $object = $arguments['object']; diff --git a/typo3/sysext/beuser/Classes/ViewHelpers/SwitchUserViewHelper.php b/typo3/sysext/beuser/Classes/ViewHelpers/SwitchUserViewHelper.php index 279fb7b86e7a..e98f426dcd5c 100644 --- a/typo3/sysext/beuser/Classes/ViewHelpers/SwitchUserViewHelper.php +++ b/typo3/sysext/beuser/Classes/ViewHelpers/SwitchUserViewHelper.php @@ -50,6 +50,8 @@ final class SwitchUserViewHelper extends AbstractViewHelper /** * Render link with sprite icon to change current backend user to target + * + * @param array{backendUser: BackendUser} $arguments */ public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext): string { -- GitLab