Skip to content

Commit 132c950

Browse files
committed
[TASK] Streamline DataHandler permission checks
The DataHandler permission check related methods - mainly those related to recordInfoWithPermissionCheck() are very hard to call correctly and contain lots of complex details. They are the source of many database queries when changing records in the Backend. Earlier attempts to improve the situation added various caches, even though caching db state in DataHandler is not a great solution since it can easily lead to side effects when records are changed multiple times in one process. The patch streamlines this area within the DataHandler significantly: The main strategy is to move the "getting record facts" (usually using BackendUtility::getRecord()) code up into the calling method and hand these records down to the permission check methods so they don't need to fetch the record in question over and over again. This aligns well with the general strategy of having the record fetching code in the top level "entry" methods (like moveRecord(), deleteRecord(), ...) and handing them around. This makes everything much more deterministic, quicker and maintainable. The permission check methods typically look at page record details and check for CONTENT_EDIT flag, ownerships and similar. The calling code thus has to fetch the page row a record is attached to. This is relatively straight, even though the code has to deal with records on pid 0 (zero) correctly. This refactoring drops four cache properties. The patch saves quite a few queries. Depending on what is measured, it seems the number of queries shrink by up to 1/3, especially with non-admin users. The v13 backport keeps the old public methods with their signature as-is to not risk breaking hooks. Resolves: #106382 Related: #106285 Releases: main, 13.4 Change-Id: I28eaf7cbf43c679bb0cad8f72be01f02d8bdd8bf Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/88445 Reviewed-by: Benni Mack <benni@typo3.org> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Benni Mack <benni@typo3.org> Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
1 parent 38e5588 commit 132c950

File tree

5 files changed

+456
-588
lines changed

5 files changed

+456
-588
lines changed

Build/phpstan/phpstan-baseline.neon

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -768,12 +768,6 @@ parameters:
768768
count: 1
769769
path: ../../typo3/sysext/core/Classes/DataHandling/DataHandler.php
770770

771-
-
772-
message: '#^Call to function is_array\(\) with list\<array\<string, mixed\>\> will always evaluate to true\.$#'
773-
identifier: function.alreadyNarrowedType
774-
count: 3
775-
path: ../../typo3/sysext/core/Classes/DataHandling/DataHandler.php
776-
777771
-
778772
message: '#^Call to function is_array\(\) with non\-empty\-array will always evaluate to true\.$#'
779773
identifier: function.alreadyNarrowedType

typo3/sysext/core/Classes/Authentication/BackendUserAuthentication.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,7 @@ public function isInWebMount($idOrRow, $readPerms = '')
341341
$checkRec = BackendUtility::getRecord(
342342
'pages',
343343
$id,
344-
't3ver_oid,'
345-
. $GLOBALS['TCA']['pages']['ctrl']['transOrigPointerField'] . ','
346-
. $GLOBALS['TCA']['pages']['ctrl']['languageField']
344+
't3ver_oid,' . $GLOBALS['TCA']['pages']['ctrl']['transOrigPointerField'] . ',' . $GLOBALS['TCA']['pages']['ctrl']['languageField']
347345
);
348346
}
349347
if ((int)($checkRec['t3ver_oid'] ?? 0) > 0) {
@@ -642,8 +640,8 @@ public function checkFullLanguagesAccess($table, $record)
642640

643641
/**
644642
* Checking if a user has editing access to a record from a $GLOBALS['TCA'] table.
645-
* The checks does not take page permissions and other "environmental" things into account.
646-
* It only deal with record internals; If any values in the record fields disallows it.
643+
* The checks do not take page permissions and other "environmental" things into account.
644+
* It only deals with record internals; If any values in the record fields disallows it.
647645
* For instance languages settings, authMode selector boxes are evaluated (and maybe more in the future).
648646
* It will check for workspace dependent access.
649647
* The function takes an ID (int) or row (array) as second argument.

0 commit comments

Comments
 (0)