Problem/Motivation

After #3339905: Add page_top and page_bottom to #attached to get rid of NodeThemeHooks::pageTop(), node preview adds the preview form through:

$build['#attached']['page_top']['node_preview']

On a Drupal 11.4.1 site, node preview can fail with:

Error: Recursion detected in array_merge_recursive() (line 165 of core/lib/Drupal/Core/Render/BubbleableMetadata.php).

The failure happens in:

Drupal\Core\Render\BubbleableMetadata::mergeAttachments()

Debugging showed that both attachment arrays being merged contained the same keyed page region attachment:

#attached['page_top']['node_preview']

Because mergeAttachments() currently falls back to array_merge_recursive() for page_top and page_bottom, duplicate render arrays in those keys can trigger PHP recursion detection.

This does not appear to be content recursion. The recursion is in the attachment merge, not repeated entity rendering.

Steps to reproduce

Observed on Drupal core 11.4.1.

1. Have a node type with preview enabled.
2. Use a full view display with nested/rendered content, for example paragraphs/entity reference revisions and Display Suite.
3. Edit a node of that type.
4. Click Preview.
5. The preview route, e.g. /node/preview/{uuid}/full, returns a 500 error.

Observed error:

Error: Recursion detected in array_merge_recursive() (line 165 of core/lib/Drupal/Core/Render/BubbleableMetadata.php).
Drupal\Core\Render\BubbleableMetadata::mergeAttachments()
Drupal\Core\Render\BubbleableMetadata->merge()
Drupal\Core\Render\RenderContext->update()
Drupal\Core\Render\Renderer->doRender()
Drupal\Core\Render\MainContent\HtmlRenderer->prepare()

In the failing request, the duplicate attachment was:

#attached['page_top']['node_preview']

Proposed resolution

Handle page_top and page_bottom in BubbleableMetadata::mergeAttachments() before the generic array_merge_recursive(), similar to the existing special handling for placeholders.

These attachment keys are keyed page-region render arrays and should not be recursively merged when the same key exists on both sides.

Remaining tasks

  • Add or confirm a failing test case for duplicate #attached['page_top'] attachments.
  • Review whether page_bottom needs identical handling.
  • Review whether keyed union is the correct merge behavior for duplicate page region children.

User interface changes

None.

Introduced terminology

None.

API changes

No public API changes intended.

Data model changes

None.

Release notes snippet

Fixes a fatal error on node preview where duplicate #attached['page_top'] attachments could cause recursion during render attachment merging.

Issue fork drupal-3611594

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

brandonlira created an issue. See original summary.

brandonlira’s picture

Attached patch prevents page_top/page_bottom attachments from being recursively merged by array_merge_recursive().

This fixed the node preview fatal locally on Drupal 11.4.1 where both attachment arrays contained #attached['page_top']['node_preview'].

Cheers!

brandonlira’s picture

Status: Active » Needs review
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new1.09 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

brandonlira’s picture

Status: Needs work » Needs review
brandonlira’s picture

Uploaded an updated patch.

smustgrave’s picture

Status: Needs review » Needs work

Thanks for reporting but as tagged this will need test coverage for sure.

brandonlira’s picture

Status: Needs work » Needs review

Hi @smustgrave,

I added test coverage for this case in BubbleableMetadataTest. The new test covers recursive render arrays in duplicate page_top/page_bottom attachments and is passing in the PHPUnit Unit (Core) job.

Could you please take another look when you have a chance?

Let me know if you think anything else should be adjusted.

smustgrave’s picture

Title: Recursion detected when merging duplicate #attached page_top/page_bottom attachments on node preview » [Regression] Recursion detected when merging duplicate #attached page_top/page_bottom attachments on node preview
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Removing tests tag based on https://git.drupalcode.org/issue/drupal-3611594/-/jobs/11131240 and manually reviewing the test.

admittedly I'm not great with this section of code but it appears to be well documented, change itself isn't massive, and in theory makes sense.

Going to mark and see what committers think.

quietone’s picture

Title: [Regression] Recursion detected when merging duplicate #attached page_top/page_bottom attachments on node preview » [regression] Recursion detected when merging duplicate #attached page_top/page_bottom attachments on node preview
Issue tags: -Regression
godotislate’s picture

Status: Reviewed & tested by the community » Needs work

1 comment on the MR.

brandonlira’s picture

Status: Needs work » Needs review

Thanks @godotislate, that makes sense. I applied the suggested simplification.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @godoislate, feedback appears to be addressed.

  • godotislate committed 902684eb on main
    fix: #3611594 [regression] Recursion detected when merging duplicate #...

  • godotislate committed 5e984898 on 11.x
    fix: #3611594 [regression] Recursion detected when merging duplicate #...

  • godotislate committed 54394781 on 11.4.x
    fix: #3611594 [regression] Recursion detected when merging duplicate #...
godotislate’s picture

Version: main » 11.4.x-dev

Test only fails as expected: https://git.drupalcode.org/project/drupal/-/jobs/11407530

Consulted with @catch to confirm this is a regression and should be backported to 11.4.x.

Committed and pushed t902684e to main, 5e98489 to 11.x, and 5439478 to 11.4.x. Thanks!

godotislate’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.