Self-nested blocks causing infinite recursion and crashing the editor

[yuliyan]

Description

With the changes in #65700, self-nested or circular-nested blocks could cause infinite recursion resulting in crashing the editor.

cc: @youknowriad

Step-by-step reproduction instructions

1. Register self-nested or circular-nested blocks. For example:

// Self-nested block
window.wp.blocks.registerBlockType( 'test/self-nested-block', {
	title: 'Self-nested Block',
	parent: [ 'test/self-nested-block', 'core/group' ],
	save: () => {
		return '';
	}
} );

// Circular-nested blocks
window.wp.blocks.registerBlockType( 'test/self-nested-block-1', {
	title: 'Self-nested Block #1',
	parent: [ 'test/self-nested-block-2', 'core/group' ],
	save: () => {
		return '';
	}
} );

window.wp.blocks.registerBlockType( 'test/self-nested-block-2', {
	title: 'Self-nested Block #2',
	parent: [ 'test/self-nested-block-1' ],
	save: () => {
		return '';
	}
} );

2. Attempt to open the Inserter from the header toolbar. Note that if the problematic blocks already exist in the page content, the block editor will immediately crash after registering the blocks.

Note that if a visible parent block comes first, e.g. [ 'core/group', 'test/self-nested-block-2' ] instead of [ 'test/self-nested-block-2', 'core/group' ], the infinite recursion will not occur.

Screenshots, screen recording, code snippet

No response

Environment info

• WordPress 6.6.2
• Gutenberg 19.4.0

Please confirm that you have searched existing issues in the repo.

• Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

• Yes

[t-hamano]

Thanks for the report.

As for Self-nested block, it seems that the problem can be solved by checking that the parents are not the same block:

diff --git a/packages/block-editor/src/store/selectors.js b/packages/block-editor/src/store/selectors.js
index a81d88de33..7409e88dbf 100644
--- a/packages/block-editor/src/store/selectors.js
+++ b/packages/block-editor/src/store/selectors.js
@@ -1584,7 +1584,8 @@ const isBlockVisibleInTheInserter = ( state, blockNameOrType ) => {
        if ( !! blockType.parent?.length ) {
                return blockType.parent.some(
                        ( name ) =>
-                               isBlockVisibleInTheInserter( state, name ) ||
+                               ( blockName !== name &&
+                                       isBlockVisibleInTheInserter( state, name ) ) ||
                                // Exception for blocks with post-content parent,
                                // the root level is often consider as "core/post-content".
                                // This exception should only apply to the post editor ideally though.

However, I still don't have a good idea of ​​how to deal with Circular-nested blocks.

[yuliyan]

@t-hamano, something like this could work, although it's not a very clean solution:

--- a/packages/block-editor/src/store/selectors.js
+++ b/packages/block-editor/src/store/selectors.js
@@ -1554,7 +1554,7 @@ export function getTemplateLock( state, rootClientId ) {
  *
  * @return {boolean} Whether the given block type is allowed to be inserted.
  */
-const isBlockVisibleInTheInserter = ( state, blockNameOrType ) => {
+const isBlockVisibleInTheInserter = ( state, blockNameOrType, checkedBlocks = new Set() ) => {
        let blockType;
        let blockName;
        if ( blockNameOrType && 'object' === typeof blockNameOrType ) {
@@ -1579,11 +1579,17 @@ const isBlockVisibleInTheInserter = ( state, blockNameOrType ) => {
                return false;
        }

+       if ( checkedBlocks.has( blockName ) ) {
+               return false;
+       }
+
+       checkedBlocks.add( blockName );
+
        // If parent blocks are not visible, child blocks should be hidden too.
        if ( !! blockType.parent?.length ) {
                return blockType.parent.some(
                        ( name ) =>
-                               isBlockVisibleInTheInserter( state, name ) ||
+                               isBlockVisibleInTheInserter( state, name, checkedBlocks ) ||
                                // Exception for blocks with post-content parent,
                                // the root level is often consider as "core/post-content".
                                // This exception should only apply to the post editor ideally though.

Edit: Updated the diff slightly.

[youknowriad]

That looks like a good approach. Anyone for a PR?

[SH4LIN]

I can handle this and will raise the PR shortly.

[SH4LIN]

Can we check the circular nesting and self-nesting at the time of registration, If circular nesting or self-nesting occurs, can we prevent block registration and throw a console error?

[youknowriad]

Good question. I guess we should try to get clarity about whether "circular nesting" is desired or not? Curious @yuliyan how you're using that at the moment?

[SH4LIN]

Blocks that are self-nested or circularly nested can never be used in the first place. For example, if a block can only be added within itself, that condition will never occur. In the case of circular nesting, both blocks would be deadlocked.

We will implement two solutions: if the parent has its own block name or if there’s a single block in circular nesting, we will stop the registration. For other cases, the previously provided solution will be applied.

[yuliyan]

Good question. I guess we should try to get clarity about whether "circular nesting" is desired or not? Curious @yuliyan how you're using that at the moment?

@youknowriad, I've discovered the issue while troubleshooting an editor crash caused by a block plugin/content-area that was restricted to inserting only in plugin/section or itself, i.e. parent: [ 'plugin/content-area', 'plugin/section' ].

As for the "circular nesting" - I don't currently use such blocks, but I thought it was also a possible use case.

Blocks that are self-nested or circularly nested can never be used in the first place. For example, if a block can only be added within itself, that condition will never occur. In the case of circular nesting, both blocks would be deadlocked.

@SH4LIN, in my examples in the issue description, the self-nested block allows to also be nested in another block that's supposedly visible, e.g. parent: [ 'test/self-nested-block', 'core/group' ]. That way, the test/self-nested-block block can first be inserted into a core/group and then nested in itself.

This is probably an edge case, but we had such block that was working prior to 19.4.0.

[youknowriad]

That seems like a valid use-case indeed. Thanks @yuliyan

[SH4LIN]

So yes, in your example, the given solution would work. However, if for some reason a developer mistakenly adds the parent attribute with its own block name, we should prevent registration and log a 'doing it wrong' error in the console.

[yuliyan]

So yes, in your example, the given solution would work. However, if for some reason a developer mistakenly adds the parent attribute with its own block name, we should prevent registration and log a 'doing it wrong' error in the console.

I agree - I doesn't make sense to have a parent attribute with just the block's own name.

[SH4LIN]

Here is the PR of the fix - #66121