Skip to content

[BUGFIX] Check BackendUtility::getRecord before using it in array_merge - #174

Open
mikelwohlschlegel wants to merge 1 commit into
IchHabRecht:mainfrom
mikelwohlschlegel:bugfix/null-check-get-record-before-array-merge
Open

[BUGFIX] Check BackendUtility::getRecord before using it in array_merge#174
mikelwohlschlegel wants to merge 1 commit into
IchHabRecht:mainfrom
mikelwohlschlegel:bugfix/null-check-get-record-before-array-merge

Conversation

@mikelwohlschlegel

Copy link
Copy Markdown

This checks BackendUtility::getRecord before using it in array_merge to avoid an exception.

We had the following problem:
There seems to be a difference in processing a history rollback between v11 und v12.

Steps to reproduce:

  • Add a new container to a page
  • Add a new content element to the same page
  • Move the new content element into the new container
  • Go back to the history and rollback ALL this steps

v11 --> WORKS
v12 --> EXCEPTION

While step debugging, I found out that $datamap['tt_content'] in

if (empty($datamap['tt_content']) || $dataHandler->bypassAccessCheckForRecords) {
is empty in v11 and not empty in v12.
My guess is, that v12 processes every single step in v12, while v11 just deletes the newly added elements (not proofed).

I'm not sure if this can maybe cause some side affects? What is the intention of merging the arrays here?

if (!empty($record)) {
$incomingFieldArray = array_merge($record, $incomingFieldArray);
}
$incomingFieldArray = array_merge(BackendUtility::getRecord('tt_content', $id), $incomingFieldArray);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems you doing the same thing twice now?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IchHabRecht You're right. I forgot to delete the original line. The commit is updated. Thanks.

@mikelwohlschlegel
mikelwohlschlegel force-pushed the bugfix/null-check-get-record-before-array-merge branch from 00a07c7 to 4503e99 Compare July 31, 2025 07:02
@Bunnyfield

Copy link
Copy Markdown

BackendUtility::getRecord() is typed to return array|null, so this call site needs the guard regardless of how the datamap ends up with an unresolved reference. We hit the same crash independently via a different workflow in L10nmgr, so this fix should be merged.

@moritz-ngo

Copy link
Copy Markdown

BackendUtility::getRecord() is typed to return array|null, so this call site needs the guard regardless of how the datamap ends up with an unresolved reference. We hit the same crash independently via a different workflow in L10nmgr, so this fix should be merged.

This PR works like charm, would be great if it could be merged

@doemedia

Copy link
Copy Markdown

I had the same issue and use the patch from moritz

@mikelwohlschlegel

Copy link
Copy Markdown
Author

Hi @IchHabRecht
three upvotes in the meanwhile. We use this patch in production since about one year. Can you think about merging it? It seems to help others...

@IchHabRecht

Copy link
Copy Markdown
Owner

Hi @mikelwohlschlegel and others,

Thank you for all your comments. I hope you understand that with the release of TYPO3 v14 and the integration of this functionality into the TYPO3 core, I will no longer be able to continue maintaining and supporting the content_defender extension in my spare time.

If there is still a need for a maintained version of content_defender, please feel free to get in touch with me so we can discuss possible options.

@mikelwohlschlegel

Copy link
Copy Markdown
Author

@IchHabRecht Thanks for your reply. This will be fully integrated into the core in v14? I did not know that. I guess, we can live with a patch till v14 then. Thanks for your effort on this.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants