Translated record gets random tx_flux_parent and may cause data loss

[dmitryd]

Related issues: #1125, #1163, #1120

The code in ContentService::initializeRecordByNewAndOldAndLanguageUids selects a random content element as a parent for translated content:

protected function initializeRecordByNewAndOldAndLanguageUids(
    $row,
    $newUid,
    $oldUid,
    $newLanguageUid,
    $languageFieldName,
    DataHandler $tceMain
) {
    if (0 < $newUid && 0 < $oldUid && 0 < $newLanguageUid) {
        $oldRecord = $this->loadRecordFromDatabase($oldUid);
        if ($oldRecord[$languageFieldName] !== $newLanguageUid && $oldRecord['pid'] === $row['pid']) {
            // look for the translated version of the parent record indicated
            // in this new, translated record. Below, we adjust the parent UID
            // so it has the UID of the translated parent if one exists.
            $translatedParents = (array) $this->workspacesAwareRecordService->get(
                'tt_content',
                'uid,sys_language_uid',
                "t3_origuid = '" . $oldRecord['tx_flux_parent'] . "'" . BackendUtility::deleteClause('tt_content')
            );
            foreach ($translatedParents as $translatedParent) {
                if ($translatedParent['sys_language_uid'] == $newLanguageUid) {
                    // set $translatedParent to the right language ($newLanguageUid):
                    break;
                }
                unset($translatedParent);
            }
            $sortbyFieldName = true === isset($GLOBALS['TCA']['tt_content']['ctrl']['sortby']) ?
                $GLOBALS['TCA']['tt_content']['ctrl']['sortby'] : 'sorting';
            $overrideValues = [
                $sortbyFieldName => $tceMain->resorting('tt_content', $row['pid'], $sortbyFieldName, $oldUid),
                'tx_flux_parent' => $translatedParent ? $translatedParent['uid'] : $oldRecord['tx_flux_parent']
            ];
            $this->updateRecordInDatabase($overrideValues, $newUid);
        }
    }
}

That for each cycle only checks the language. Since tx_flux_parent is zero, it selects all content elements. So any content elements with a proper sys_language_uid field will be selected as a parent for the translated content element, which is obviously wrong.

Now the worse part: delete that randomly selected parent and all translated elements that refer to it, will be also deleted because TYPO3 thinks they are inline children of that random parent element due to $TCA configuration of tx_flux_parent. This way we lost 197 translated elements by deleting one hidden unused element on a hidden unused test page.

I am not 100% sure if I did the right thing (so no PR) but the following seems to work for me:

protected function initializeRecordByNewAndOldAndLanguageUids(
     $row,
     $newUid,
     $oldUid,
     $newLanguageUid,
     $languageFieldName,
     DataHandler $tceMain
 ) {
     if (0 < $newUid && 0 < $oldUid && 0 < $newLanguageUid) {
         $oldRecord = $this->loadRecordFromDatabase($oldUid);
         if ($oldRecord[$languageFieldName] !== $newLanguageUid && $oldRecord['pid'] === $row['pid'] && MathUtility::canBeInterpretedAsInteger($oldRecord['tx_flux_parent']) && $oldRecord['tx_flux_parent'] > 0) {
             // look for the translated version of the parent record indicated
             // in this new, translated record. Below, we adjust the parent UID
             // so it has the UID of the translated parent if one exists.
             $translatedParents = (array) $this->workspacesAwareRecordService->get(
                'tt_content',
                'uid,sys_language_uid',
                'sys_language_uid IN (-1,0,' . (int)$newLanguageUid . ') AND ' .
                'pid=' . (int)$row['pid'] . ' AND ' .
                'tx_flux_parent=' . $oldRecord['tx_flux_parent'] . BackendUtility::deleteClause('tt_content'),
                 '', 'sys_language_uid DESC'
             );
             $translatedParent = null;
             foreach ($translatedParents as $translatedParent) {
                 if ($translatedParent['sys_language_uid'] == $newLanguageUid) {
                     // set $translatedParent to the right language ($newLanguageUid):
                     break;
                 }
                 unset($translatedParent);
             }
             $sortbyFieldName = true === isset($GLOBALS['TCA']['tt_content']['ctrl']['sortby']) ?
                 $GLOBALS['TCA']['tt_content']['ctrl']['sortby'] : 'sorting';
             $overrideValues = [
                 $sortbyFieldName => $tceMain->resorting('tt_content', $row['pid'], $sortbyFieldName, $oldUid),
                 'tx_flux_parent' => $translatedParent ? $translatedParent['uid'] : $oldRecord['tx_flux_parent']
             ];
             $this->updateRecordInDatabase($overrideValues, $newUid);
         }
     }
 }

[dmitryd]

Probably it should also be sorted by hidden ASC to get visible elements first.

[NamelessCoder]

This way we lost 197 translated elements by deleting one hidden unused element on a hidden unused test page.

Holy crudbasket... I'm sorry about that Dmitry. Thanks for the excellent description and suggested fix, I hope to get some time one of the coming days to coordinate this along with #1163.

[dmitryd]

We restored from the backup :) But we will make an xclass ext for all projects to avoid this issue. If you can fix it for the next release, it would be great :)

[ghost]

Any Updates on this ? We ran into the same issue, delete one record and it wipes out all translations for 10 pages.

[NamelessCoder]

Sorry guys, I'm under a bit of pressure from other tasks at the moment. I haven't forgotten.

[DenisMir]

Is this issue valid for older flux versions as well or only the latest one?

[jochenrieger]

Hi all,

it seems we ran into the same issue in two projects this year (both based on T3 7 LTS), using flux based content elements. One time in February using the actual version at the time (don't remember exactly which one).

And just last Thursday (22nd Sep) using flux 7.3.0.

Within 4 seconds all translated content elements from around 45 pages got deleted.

That's indeed a tough one. Thanks for any efforts to fix this and Dmitry, thanks for that great analysis. I, so far, had no clue how to reproduce and debug the weird behavior.

Best wishes
Jochen

[dmitryd]

@DenisMir

Is this issue valid for older flux versions as well or only the latest one?

Some old versions also. I think we discovered it in an old 6.2 project.

[pixelbrackets]

Hey guys,

I experienced this bug as well. It may cause data loss or permanent damanges to the dataset.

I'll explain what happend, what our analysis was, send a pull request for a bugfix and how to fix existing installations.

What happend

Client tells us, that several translated records are missing since two days. Not all records however. Which sounds really odd.

We investigate the issue and stumble upon the fact, that lots of record on a given example page got deleted on the same time.

When I take a look at the TYPO3 log I see that 1168 records got deleted at once. The records had no relation to each other (like same type, pid, language, creation time, editor etc.). The only common thing was, that they are translations. But as I said, not all translations got deleted.

Our analysis

Fortunately the editor did not do very much in the moment the bug occured and therefore remembered what he deleted first. I was able to reproduce the mass deletion, when I repeated his actions on a backup machine.

We do use TYPO3 6.2 and Flux 7.2.3.

When a record is deleted TYPO3 calls the »deleteRecord« method in »sysext/core/Classes/DataHandling/DataHandler.php«. This method reads the TCA of the current table and checks if the record has any references such as inline type or MM references. If so, these child records also to be deleted.

In the debug trace the array of the elements to be deleted increased rapidly. The first loop had one field value in common: the same UID in the »tx_flux_parent« field.

So we knew that not the delete method was wrong but our data.

We then searched for methods were this field is set. The »initializeRecordByNewAndOldAndLanguageUids« method in »EXT:flux/Classes/Service/ContentService.php« was responsible. While translating a record this method looks for the latest version of a flux parent record. A Flux parent is assigned to every record which is nested into fluid content element (the parent).

If the origin of the translated record has a reference to a flux parent, then the search will return a usefull result.
But if the origin has no flux parent, then the search causes this SQL statement:

SELECT uid FROM tt_content WHERE t3_origuid = 0 AND tt_content.deleted=0

This will return a huge array of course. The method selects the first item in the array as parent (*¹). So the item is rather random (It may be the same for some while, but may change due to mysql updates or the moon phase due to missing MySQL sorting params).

The method then assigns the wrong parent to the transalted record.

In our case the wrong parent was assigned to almost 2000 records.

Once this single, random record gets deleted TYPO3 will delete all of these records. The fact that not all got deleted in our installation is caused by the fact that the maximum execution time of the process was exceeded.

Pull request

I have read Dmitrys buxfix draft (heads up, thank you again for your OS commitment btw), tested it, build a somewhat shorter patch and changed the relevant unit test.

Example records in my test installation:

Records in original langauge

uid,header,sys_language_uid,tx_flux_parent
39999,Text Right,0,39997
39998,Text Left,0,39997
39997,Fluid Content - Two Columns,0,0
39996,Regular Text,0,0

Translated records without Bugfix

uid,header,sys_language_uid,tx_flux_parent
40003,[Translate to English:] Fluid Content - Two Columns,1,5154
40002,[Translate to English:] Text Left,1,40003
40001,[Translate to English:] Text Right,1,40003
40000,[Translate to English:] Regular Text,1,5154

Translated records with Bugfix

uid,header,sys_language_uid,tx_flux_parent
40003,[Translate to English:] Fluid Content - Two Columns,1,0
40002,[Translate to English:] Text Left,1,40003
40001,[Translate to English:] Text Right,1,40003
40000,[Translate to English:] Regular Text,1,0

Due to the impact of the bug I will send a PR for the master and legacy branch.

Fix existing installations

Only relevant for installations with translations.

1. Update to the flux extension as soon as the PR is merged
2. Fix the records in your database manually

The Bugfix is not enough, since records already may have faulty references to the random record. Once thhis random record is deleted by an editor, all references records will be deleted as well.

To find all records which may have wrong references I used statements like these:

# Original wrong select statement, which returns the same element uid as flux used until the bugfix
SELECT uid FROM tt_content WHERE t3_origuid = 0 AND tt_content.deleted = 0;
# If you have several translated records already, then the parent assigned the most is probably a wrong one
SELECT COUNT(uid) as counter, tx_flux_parent FROM tt_content WHERE sys_language_uid > 0 AND tx_flux_parent > 0 GROUP BY tx_flux_parent ORDER BY counter DESC;
# Manually check the selected records for plausibility (e.g. pid, maybe langauge) (*²)
# Update/Fix the existing records
UPDATE `tt_content` SET `tx_flux_parent` = '0' WHERE `tx_flux_parent` = 'XXXX' AND sys_language_uid > 0;

@NamelessCoder maybe a update script in the extension could do this automatically. But such a automatization may be risky as well. I would like to help, but I am not available until Monday.

Please note as well: restoring a backup is the best method to restore the data if you are alerted soon enough. If you use the extension »recycler« in the TYPO3 backend, then you may restore items which had been deleted quite rightly previously. This is due to the fact that the receycler uses the timestamp of the record. During the unwantd mass deletion this timestamp get updated even if the child element was deleted previously. So this method is no safe way to restore the data, but creates inconsistent data and therefore new issues instead.

Edit:
*¹ In Flux 7.2.3,the latest version for TYPO3 6.2, the first item in the array is selected. In Flux 7.4.0, the latest version for TYPO3 7, the first item in the array which has the same language as the current record is selected.

*²

# …or determine that you have a standard installation, where nesting of elements with flux is not used for any other purpose than fluid content elements. As @BenjaminBeck pointed out, we then may assume that the field »tx_flux_column« in the »tt_content« table is always set if a valid flux parent exists for the record. Therefore all elements with a flux parent but an empty flux column are corrupt. Get a list of those
SELECT COUNT(uid) as counter, tx_flux_parent FROM tt_content WHERE sys_language_uid > 0 AND tx_flux_parent > 0 AND `tx_flux_column` = '' GROUP BY tx_flux_parent ORDER BY counter DESC;
# …the short version - without any manual checks - to clean up the database
UPDATE `tt_content` SET `tx_flux_parent` = '0' WHERE `tx_flux_parent` > 0 AND `tx_flux_column` = '';

[jochenrieger]

@pixelbrackets Double thumbs up for this one. Amazing Analysis!

[BenjaminBeck]

Hi together
I think i have a good solution for a one line sql-fix:

update tt_content set tx_flux_parent = null where tx_flux_parent > 0 and tx_flux_column = '';

This code assumes that a record that has a empty tx_flux_column and a tx_flux_parent > 0 is an invalid record that only can come from this bug. (That is at least the case in my installation)

@NamelessCoder if that assumption is right, this may be sql for the update-script?

[dmitryd]

@pixelbrackets

I have read Dmitrys buxfix draft (heads up, thank you again for your OS commitment btw), tested it, build a somewhat shorter patc

From the quick look it seems to be identical to mine, not shorter. Just a condition change.

[pixelbrackets]

Hey,

I have edited the analysis and added two remarks (different flux parent in TYPO3 6.2 and 7, fix for existing dataset by @BenjaminBeck ).

@dmitryd Yes, just the condition change. I did not include the enhanced search for translated parents in to the bugfix PR.

@NamelessCoder / Team I realized, that I did develop against master and the contribution guidlines of flux require coding against the development branch. I copied my feature branch and applied a rebase to it: https://github.com/webit-de/typo3-flux/tree/20161013_prevent-random-parent-uid-for-translations-development-rebase
Should I send another PR and abondon the current one?

Could you please give a feedback about the assumption in point *² of the analysis post. I would like to run this query as soon as possible on all my projects.

[NamelessCoder]

@pixelbrackets sorry I haven't had time to go through all of it. But:

Yes, please recreate this targeting the development branch (and another for the legacy branch if you wish).

Yes, the assumption you make above is correct. Any live record (not just fluidcontent) with a Flux parent and without a column name would be incorrect.

Yes, the proposed SQL fixes should work. Please integrate them as an extension update script option and add checks so it is only proposed if corrupt records are detected. Putting it here makes it a very conscious decision when to run it (so sysadmin can back up before).

[pixelbrackets]

Cool, thanks for the feedback.

I'll do so and send the proper PRs.

Ah, thanks for the confirmation, that helps a lot.

I will write such an update script and let you know about the progress.

edit: update script done

[EdRoxter]

In case there is a release timeline that I just didn't find, I apologize for the question - but is there an outlook when this issue will be resolved in the TER upstream version?

[NamelessCoder]

Notes are on https://fluidtypo3.org/blog/news/preparing-flux-750.html (you'd be forgiven for not keeping up with that feed, it gets very rare updates).

[pixelbrackets]

PSA: Sorry for the long commit list appending to this issue, I have rebased the PR branch a couple of times (#1188).

Claus cherry-picked and tweaked the commits into the development branch this night. 😃 ✅

[tmotyl]

FYI, for TYPO3 v8, the usage of the t3_origuid should be changed to newly introduced l10n_source field.
As there are known issues resulting in wrong values of t3_origuid, e.g. if you edit a record in workspaces.
see https://docs.google.com/document/d/1rv_P8nO1F2VvBEk3Tbb5zH6LuCXHUsgMo0csde6NGUQ/edit#
and https://review.typo3.org/#/c/51070/26/typo3/sysext/core/Documentation/Changelog/master/Feature-78169-IntroduceTranslationSourceFieldForTt_content.rst
for more context.

[ghost]

Any news when we can expect an bugfix for 7.6.X or is there any blocker that holds us back till upcoming release of 8 LTS ?

[ghost]

I've just found the changelog for flux 8.0.0, there i can see the mentioned commit by pixelbrackets here as its tagged with 8.0.0 and also 8.0.1 this was already released ? So someone missed only to mention this bugfix and close this issue ?

[pixelbrackets]

@verdure-peuser Yes, the fix was cherry-picked to the development branch in January, the development branch then was released on January 9th → https://github.com/FluidTYPO3/flux/releases/tag/8.0.0 (https://github.com/FluidTYPO3/flux/blob/development/Documentation/Changelog/8.0.0.md)

So yes, this ticket may be closed.