Skip to content

Commit

Permalink
[TASK] Avoid GU::_GP() in DataHandler
Browse files Browse the repository at this point in the history 
DataHandler should of course *not* have a dependency
to _GET or _POST: All data should be hand over as
"command" and "data" array.

Flex form handling violates this for section container
deletion and sorting by using GeneralUtility::_GP().

The patch changes the logic to carry the "_ACTION"
information within the flex data array. This simplifies
the flex form handling logic a bit, we can get rid
of a recursion along the way.

Resolves: #99952
Releases: main
Change-Id: Ifaa15162d018725fe3ba8ae7b1a05655a0cffb36
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77856
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
  • Loading branch information
@lolli42
lolli42 committed Feb 15, 2023
1 parent 51bef53 commit 7e57642
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 215 deletions.
5 changes: 0 additions & 5 deletions Build/phpstan/phpstan-baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,11 +395,6 @@ parameters:
count: 1
path: ../../typo3/sysext/core/Classes/DataHandling/DataHandler.php

-
message: "#^Result of \\|\\| is always false\\.$#"
count: 1
path: ../../typo3/sysext/core/Classes/DataHandling/DataHandler.php

-
message: "#^Variable \\$fieldArray in empty\\(\\) always exists and is not falsy\\.$#"
count: 1
Expand Down
12 changes: 5 additions & 7 deletions typo3/sysext/backend/Classes/Form/Container/FlexFormContainerContainer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,13 @@ public function render()
$flexFormFormPrefix = $this->data['flexFormFormPrefix'];
$flexFormContainerElementCollapsed = $this->data['flexFormContainerElementCollapsed'];
$flexFormDataStructureArray = $this->data['flexFormDataStructureArray'];
$parameterArray = $this->data['parameterArray'];

$iconFactory = GeneralUtility::makeInstance(IconFactory::class);
$flexFormContainerIdentifier = $this->data['flexFormContainerIdentifier'];
$actionFieldName = '_ACTION_FLEX_FORM'
. $parameterArray['itemFormElName']
. $this->data['flexFormFormPrefix']
. '[_ACTION]'
. '[' . $flexFormContainerIdentifier . ']';
$actionFieldName = 'data[' . $table . '][' . $row['uid'] . '][' . $fieldName . ']'
. $flexFormFormPrefix
. '[' . $flexFormContainerIdentifier . ']'
. '[_ACTION]';
$toggleFieldName = 'data[' . $table . '][' . $row['uid'] . '][' . $fieldName . ']'
. $flexFormFormPrefix
. '[' . $flexFormContainerIdentifier . ']'
Expand Down Expand Up @@ -101,7 +99,6 @@ public function render()

$html = [];
$html[] = '<div ' . GeneralUtility::implodeAttributes($containerAttributes, true) . '>';
$html[] = '<input class="t3js-flex-control-action" type="hidden" name="' . htmlspecialchars($actionFieldName) . '" value="" />';
$html[] = '<div ' . GeneralUtility::implodeAttributes($panelHeaderAttributes, true) . '>';
$html[] = '<div class="form-irre-header ' . ($flexFormContainerElementCollapsed ? ' collapsed' : '') . '">';
$html[] = '<div class="form-irre-header-cell form-irre-header-icon">';
Expand All @@ -123,6 +120,7 @@ public function render()
$html[] = '<div id="' . htmlspecialchars($flexFormDomContainerId) . '" class="panel-collapse collapse t3js-flex-section-content ' . ($flexFormContainerElementCollapsed ? '' : 'show') . '">';
$html[] = $containerContentResult['html'];
$html[] = '</div>';
$html[] = '<input class="t3js-flex-control-action" type="hidden" name="' . htmlspecialchars($actionFieldName) . '" value="" />';
$html[] = '<input';
$html[] = 'class="t3js-flex-control-toggle"';
$html[] = 'type="hidden"';
Expand Down
212 changes: 96 additions & 116 deletions typo3/sysext/core/Classes/DataHandling/DataHandler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2399,151 +2399,130 @@ protected function applyFiltersToValues(array $tcaFieldConfiguration, array $val
*/
protected function checkValueForFlex($res, $value, $tcaFieldConf, $table, $id, $curValue, $status, $realPid, $recFID, $tscPID, $field)
{
if (is_array($value)) {
// This value is necessary for flex form processing to happen on flexform fields in page records when they are copied.
// Problem: when copying a page, flexform XML comes along in the array for the new record - but since $this->checkValue_currentRecord
// does not have a uid or pid for that sake, the FlexFormTools->getDataStructureIdentifier() function returns no good DS. For new
// records we do know the expected PID so therefore we send that with this special parameter. Only active when larger than zero.
$row = $this->checkValue_currentRecord;
if ($status === 'new') {
$row['pid'] = $realPid;
}
if (!is_array($value)) {
$res['value'] = $value;
return $res;
}

$flexFormTools = GeneralUtility::makeInstance(FlexFormTools::class);
// This value is necessary for flex form processing to happen on flexform fields in page records when they are copied.
// Problem: when copying a page, flexform XML comes along in the array for the new record - but since $this->checkValue_currentRecord
// does not have a uid or pid for that sake, the FlexFormTools->getDataStructureIdentifier() function returns no good DS. For new
// records we do know the expected PID, so we send that with this special parameter. Only active when larger than zero.
$row = $this->checkValue_currentRecord;
if ($status === 'new') {
$row['pid'] = $realPid;
}

// Get data structure. The methods may throw various exceptions, with some of them being
// ok in certain scenarios, for instance on new record rows. Those are ok to "eat" here
// and substitute with a dummy DS.
// Get data structure. The methods may throw various exceptions, with some of them being
// ok in certain scenarios, for instance on new record rows. Those are ok to "eat" here
// and substitute with a dummy DS.
try {
$flexFormTools = GeneralUtility::makeInstance(FlexFormTools::class);
$dataStructureIdentifier = $flexFormTools->getDataStructureIdentifier(
['config' => $tcaFieldConf],
$table,
$field,
$row
);
$dataStructureArray = $flexFormTools->parseDataStructureByIdentifier($dataStructureIdentifier);
} catch (InvalidParentRowException|InvalidParentRowLoopException|InvalidParentRowRootException|InvalidPointerFieldValueException|InvalidIdentifierException) {
$dataStructureArray = ['sheets' => ['sDEF' => []]];
try {
$dataStructureIdentifier = $flexFormTools->getDataStructureIdentifier(
['config' => $tcaFieldConf],
$table,
$field,
$row
);

$dataStructureArray = $flexFormTools->parseDataStructureByIdentifier($dataStructureIdentifier);
} catch (InvalidParentRowException|InvalidParentRowLoopException|InvalidParentRowRootException|InvalidPointerFieldValueException|InvalidIdentifierException $e) {
}
}

// Get current value array:
$currentValueArray = (string)$curValue !== '' ? GeneralUtility::xml2array($curValue) : [];
if (!is_array($currentValueArray)) {
$currentValueArray = [];
}
// Remove all old meta for languages...
// Evaluation of input values:
$value['data'] = $this->checkValue_flex_procInData($value['data'] ?? [], $currentValueArray['data'] ?? [], $dataStructureArray, [$table, $id, $curValue, $status, $realPid, $recFID, $tscPID]);
// Create XML from input value:
$xmlValue = $this->checkValue_flexArray2Xml($value, true);
// Get current value array:
$currentValueArray = (string)$curValue !== '' ? GeneralUtility::xml2array($curValue) : [];
if (!is_array($currentValueArray)) {
$currentValueArray = [];
}
// Remove all old meta for languages...
// Evaluation of input values:
$value['data'] = $this->checkValue_flex_procInData($value['data'] ?? [], $currentValueArray['data'] ?? [], $dataStructureArray, [$table, $id, $curValue, $status, $realPid, $recFID, $tscPID]);
// Create XML from input value:
$xmlValue = $this->checkValue_flexArray2Xml($value);

// Here we convert the currently submitted values BACK to an array, then merge the two and then BACK to XML again. This is needed to ensure the charsets are the same
// (provided that the current value was already stored IN the charset that the new value is converted to).
$arrValue = GeneralUtility::xml2array($xmlValue);
// Here we convert the currently submitted values BACK to an array, then merge the two and then BACK to XML again. This is needed to ensure the charsets are the same
// (provided that the current value was already stored IN the charset that the new value is converted to).
$xmlAsArray = GeneralUtility::xml2array($xmlValue);

foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['checkFlexFormValue'] ?? [] as $className) {
$hookObject = GeneralUtility::makeInstance($className);
if (method_exists($hookObject, 'checkFlexFormValue_beforeMerge')) {
$hookObject->checkFlexFormValue_beforeMerge($this, $currentValueArray, $arrValue);
}
foreach ($GLOBALS['TYPO3_CONF_VARS']['SC_OPTIONS']['t3lib/class.t3lib_tcemain.php']['checkFlexFormValue'] ?? [] as $className) {
$hookObject = GeneralUtility::makeInstance($className);
if (method_exists($hookObject, 'checkFlexFormValue_beforeMerge')) {
$hookObject->checkFlexFormValue_beforeMerge($this, $currentValueArray, $xmlAsArray);
}
}

ArrayUtility::mergeRecursiveWithOverrule($currentValueArray, $arrValue);
$xmlValue = $this->checkValue_flexArray2Xml($currentValueArray, true);
ArrayUtility::mergeRecursiveWithOverrule($currentValueArray, $xmlAsArray);
$xmlValue = $this->checkValue_flexArray2Xml($currentValueArray);

// Action commands (sorting order and removals of elements) for flexform sections, see FormEngine for the use of this GP parameter.
// @todo: It is of course ugly DH fetches from POST here. The actions should be part of the
// command array (?!) instead, which requires at least FormEngine + JS adaptions.
$actionCMDs = GeneralUtility::_GP('_ACTION_FLEX_FORMdata');
$relevantId = $id;
if ($status === 'update'
&& BackendUtility::isTableWorkspaceEnabled($table)
&& (int)($row['t3ver_wsid'] ?? 0) > 0
&& (int)($row['t3ver_oid'] ?? 0) > 0
&& !is_array($actionCMDs[$table][$id][$field] ?? false)
&& is_array($actionCMDs[$table][(int)$row['t3ver_oid']][$field] ?? false)
) {
// Scenario: A record with multiple container sections exists in live. The record has no workspace overlay, yet.
// It is then edited in workspaces and sections are resorted or deleted, which should create the version overlay
// plus the resorting or deleting of sections in the version overlay record.
// FormEngine creates this '_ACTION_FLEX_FORMdata' data array with the uid of the live record, since FormEngine
// does not know the uid of the overlay record, yet.
// DataHandler first creates the new overlay record via copyRecord_raw(), which calls this method. At this point,
// we leave the new version record untouched, sorting and deletions of flex sections are not applied.
// DataHandler then calls this method a second time to apply modifications to the just created overlay record. The
// incoming $row is now the version row, and $row['uid'] und incoming $id are the versione'd record uid.
// The '_ACTION_FLEX_FORMdata' POST data however is still the uid of the live record!
// Actions are then not applied since the uid lookups don't match.
// To solve this situation we check for this scenario in the above if conditions and use the live version
// uid (t3ver_oid) to access data from the '_ACTION_FLEX_FORMdata' array.
$relevantId = (int)$row['t3ver_oid'];
}
if (is_array($actionCMDs[$table][$relevantId][$field]['data'] ?? false)) {
$arrValue = GeneralUtility::xml2array($xmlValue);
$this->_ACTION_FLEX_FORMdata($arrValue['data'], $actionCMDs[$table][$relevantId][$field]['data']);
$xmlValue = $this->checkValue_flexArray2Xml($arrValue, true);
}
// Create the value XML:
$res['value'] = $xmlValue;
} else {
// Passthrough...:
$res['value'] = $value;
}
$xmlAsArray = GeneralUtility::xml2array($xmlValue);
$xmlAsArray = $this->sortAndDeleteFlexSectionContainerElements($xmlAsArray, $dataStructureArray);
$xmlValue = $this->checkValue_flexArray2Xml($xmlAsArray);

$res['value'] = $xmlValue;
return $res;
}

/**
* Converts an array to FlexForm XML
*
* @param array $array Array with FlexForm data
* @param bool $addPrologue If set, the XML prologue is returned as well.
* @return string Input array converted to XML
* @internal should only be used from within DataHandler
*/
public function checkValue_flexArray2Xml($array, $addPrologue = false): string
public function checkValue_flexArray2Xml($array): string
{
$flexObj = GeneralUtility::makeInstance(FlexFormTools::class);
return $flexObj->flexArray2Xml($array, $addPrologue);
return $flexObj->flexArray2Xml($array, true);
}

/**
* Actions for flex form element (move, delete)
* allows to remove and move flexform sections
* Delete and resort section container elements.
*
* @param array $valueArray by reference
* @param array $actionCMDs
* @todo: It would be better if the magic _ACTION key would be a 'command array', not part of 'data array'
*/
protected function _ACTION_FLEX_FORMdata(&$valueArray, $actionCMDs)
private function sortAndDeleteFlexSectionContainerElements(array $valueArray, array $dataStructure): array
{
if (!is_array($valueArray) || !is_array($actionCMDs)) {
return;
}

foreach ($actionCMDs as $key => $value) {
if ($key === '_ACTION') {
// First, check if there are "commands":
if (empty(array_filter($actionCMDs[$key]))) {
continue;
}

asort($actionCMDs[$key]);
$newValueArray = [];
foreach ($actionCMDs[$key] as $idx => $order) {
// Just one reflection here: It is clear that when removing elements from a flexform, then we will get lost
// files unless we act on this delete operation by traversing and deleting files that were referred to.
if ($order !== 'DELETE') {
$newValueArray[$idx] = $valueArray[$idx];
foreach (($dataStructure['sheets'] ?? []) as $dataStructureSheetName => $dataStructureSheetDefinition) {
if (!isset($dataStructureSheetDefinition['ROOT']['el']) || !is_array($dataStructureSheetDefinition['ROOT']['el'])) {
continue;
}
$dataStructureFields = $dataStructureSheetDefinition['ROOT']['el'];
foreach ($dataStructureFields as $dataStructureFieldName => $dataStructureFieldDefinition) {
if (isset($dataStructureFieldDefinition['type']) && $dataStructureFieldDefinition['type'] === 'array'
&& isset($dataStructureFieldDefinition['section']) && (string)$dataStructureFieldDefinition['section'] === '1'
) {
// Found a possible section within flex form data structure definition
if (!is_array($valueArray['data'][$dataStructureSheetName]['lDEF'][$dataStructureFieldName]['el'] ?? false)) {
// No containers in data
continue;
}
unset($valueArray[$idx]);
$newElements = [];
$containerCounter = 0;
foreach ($valueArray['data'][$dataStructureSheetName]['lDEF'][$dataStructureFieldName]['el'] as $sectionKey => $sectionValues) {
// Remove to-delete containers
$action = $sectionValues['_ACTION'] ?? '';
if ($action === 'DELETE') {
continue;
}
if (($sectionValues['_ACTION'] ?? '') === '') {
$sectionValues['_ACTION'] = $containerCounter;
}
$newElements[$sectionKey] = $sectionValues;
$containerCounter++;
}
// Resort by action key
uasort($newElements, function ($a, $b) {
return (int)$a['_ACTION'] - (int)$b['_ACTION'];
});
foreach ($newElements as &$element) {
// Do not store action key
unset($element['_ACTION']);
}
$valueArray['data'][$dataStructureSheetName]['lDEF'][$dataStructureFieldName]['el'] = $newElements;
}
$valueArray += $newValueArray;
} elseif (is_array($actionCMDs[$key]) && isset($valueArray[$key])) {
// @todo: This recursion is probably obsolete since containers can't be nested into sections again.
$this->_ACTION_FLEX_FORMdata($valueArray[$key], $actionCMDs[$key]);
}
}
return $valueArray;
}

/**
Expand Down Expand Up @@ -3152,8 +3131,9 @@ public function checkValue_flex_procInData_travDS(&$dataValues, $dataValues_curr
if (!is_array($dataValues_current[$key]['el'] ?? false)) {
$dataValues_current[$key]['el'] = [];
}
// @todo: Ugly! This relies on the fact that _TOGGLE and _ACTION are *below* the business fields!
$theKey = key($el);
if (!is_array($dataValues[$key]['el'][$ik][$theKey]['el'])) {
if (!is_array($dataValues[$key]['el'][$ik][$theKey]['el'] ?? false)) {
continue;
}

Expand Down Expand Up @@ -6423,7 +6403,7 @@ public function remapListedDBRecords()
$currentValueArray['data'] = $this->checkValue_flex_procInData($currentValueArray['data'], [], $dataStructureArray, [$table, $theUidToUpdate, $fieldName], 'remapListedDBRecords_flexFormCallBack');
// The return value should be compiled back into XML, ready to insert directly in the field (as we call updateDB() directly later):
if (is_array($currentValueArray['data'])) {
$newData[$fieldName] = $this->checkValue_flexArray2Xml($currentValueArray, true);
$newData[$fieldName] = $this->checkValue_flexArray2Xml($currentValueArray);
}
}
}
Expand Down Expand Up @@ -6851,7 +6831,7 @@ protected function updateFlexFormData($flexFormId, array $modifications)
if (is_array($valueStructure['data'])) {
// The return value should be compiled back into XML
$values = [
$field => $this->checkValue_flexArray2Xml($valueStructure, true),
$field => $this->checkValue_flexArray2Xml($valueStructure),
];

$this->updateDB($table, $uid, $values);
Expand Down

0 comments on commit 7e57642

Please sign in to comment.