From 3b54ba841e55ff92570f762b4d9c3d03cbc5e71d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Greg=20Zi=C3=B3=C5=82kowski?= Date: Mon, 8 Mar 2021 15:28:08 +0100 Subject: [PATCH] Template Part: Prevent infinite recursion (#28456) * Template Part: Prevent infinite recursion Based on #28405 from @mcsf. Tries to apply the same technique used for Reusable block to prevent infinite recursion when the same block is inserted into itself at any level of nesting. * Change the order of warning messages * Prevents a block from rendering itself only when the same block type * Fix styling issues in PHP file for Template Part block * Add fixes for issues discovered while testing * Ensure immutability in useNoRecursiveRenders * Rename ref to uniqueId in the test --- .../use-no-recursive-renders/index.js | 40 +++++++-- .../test/use-no-recursive-renders.js | 84 +++++++++++++------ packages/block-library/src/block/index.php | 2 +- .../src/template-part/edit/index.js | 19 ++++- .../block-library/src/template-part/index.php | 45 ++++++++-- 5 files changed, 146 insertions(+), 44 deletions(-) diff --git a/packages/block-editor/src/components/use-no-recursive-renders/index.js b/packages/block-editor/src/components/use-no-recursive-renders/index.js index 94ae4b7c19263..75930f70b5283 100644 --- a/packages/block-editor/src/components/use-no-recursive-renders/index.js +++ b/packages/block-editor/src/components/use-no-recursive-renders/index.js @@ -8,18 +8,37 @@ import { useMemo, } from '@wordpress/element'; -const RenderedRefsContext = createContext( new Set() ); +/** + * Internal dependencies + */ +import { useBlockEditContext } from '../block-edit/context'; + +const RenderedRefsContext = createContext( {} ); + +/** + * Immutably adds an unique identifier to a set scoped for a given block type. + * + * @param {Object} renderedBlocks Rendered blocks grouped by block name + * @param {string} blockName Name of the block. + * @param {*} uniqueId Any value that acts as a unique identifier for a block instance. + * + * @return {Object} The list of rendered blocks grouped by block name. + */ +function addToBlockType( renderedBlocks, blockName, uniqueId ) { + const result = { + ...renderedBlocks, + [ blockName ]: renderedBlocks[ blockName ] + ? new Set( renderedBlocks[ blockName ] ) + : new Set(), + }; + result[ blockName ].add( uniqueId ); -// Immutably add to a Set -function add( set, element ) { - const result = new Set( set ); - result.add( element ); return result; } /** * A React hook for keeping track of blocks previously rendered up in the block - * tree. Blocks susceptible to recursiion can use this hook in their `Edit` + * tree. Blocks susceptible to recursion can use this hook in their `Edit` * function to prevent said recursion. * * @param {*} uniqueId Any value that acts as a unique identifier for a block instance. @@ -32,10 +51,13 @@ function add( set, element ) { */ export default function useNoRecursiveRenders( uniqueId ) { const previouslyRenderedBlocks = useContext( RenderedRefsContext ); - const hasAlreadyRendered = previouslyRenderedBlocks.has( uniqueId ); + const { name: blockName } = useBlockEditContext(); + const hasAlreadyRendered = Boolean( + previouslyRenderedBlocks[ blockName ]?.has( uniqueId ) + ); const newRenderedBlocks = useMemo( - () => add( previouslyRenderedBlocks, uniqueId ), - [ uniqueId, previouslyRenderedBlocks ] + () => addToBlockType( previouslyRenderedBlocks, blockName, uniqueId ), + [ previouslyRenderedBlocks, blockName, uniqueId ] ); const Provider = useCallback( ( { children } ) => ( diff --git a/packages/block-editor/src/components/use-no-recursive-renders/test/use-no-recursive-renders.js b/packages/block-editor/src/components/use-no-recursive-renders/test/use-no-recursive-renders.js index 0844dd601b3a2..be41180eb7a22 100644 --- a/packages/block-editor/src/components/use-no-recursive-renders/test/use-no-recursive-renders.js +++ b/packages/block-editor/src/components/use-no-recursive-renders/test/use-no-recursive-renders.js @@ -4,34 +4,45 @@ import { render } from '@testing-library/react'; /** - * WordPress dependencies + * Internal dependencies */ -import { Fragment } from '@wordpress/element'; -import { __experimentalUseNoRecursiveRenders as useNoRecursiveRenders } from '@wordpress/block-editor'; +import useNoRecursiveRenders from '../'; +import { + BlockEditContextProvider, + useBlockEditContext, +} from '../../block-edit/context'; // Mimics a block's Edit component, such as ReusableBlockEdit, which is capable -// of calling itself depending on its `ref` attribute. -function Edit( { attributes: { ref } } ) { +// of calling itself depending on its `uniqueId` attribute. +function Edit( { attributes: { uniqueId } } ) { + const { name } = useBlockEditContext(); const [ hasAlreadyRendered, RecursionProvider ] = useNoRecursiveRenders( - ref + uniqueId ); if ( hasAlreadyRendered ) { - return
Halt
; + return
Halt
; } return ( -
- { ref === 'SIMPLE' &&

Done

} - { ref === 'SINGLY-RECURSIVE' && ( - +
+ { uniqueId === 'SIMPLE' &&

Done

} + { uniqueId === 'SINGLY-RECURSIVE' && ( + ) } - { ref === 'MUTUALLY-RECURSIVE-1' && ( - + { uniqueId === 'ANOTHER-BLOCK-SAME-ID' && ( + + + ) } - { ref === 'MUTUALLY-RECURSIVE-2' && ( - + { uniqueId === 'MUTUALLY-RECURSIVE-1' && ( + + ) } + { uniqueId === 'MUTUALLY-RECURSIVE-2' && ( + ) }
@@ -39,9 +50,13 @@ function Edit( { attributes: { ref } } ) { } describe( 'useNoRecursiveRenders', () => { + const context = { name: 'reusable-block' }; + it( 'allows a single block to render', () => { const { container } = render( - + + + ); expect( container.querySelectorAll( '.wp-block__reusable-block' ) @@ -53,10 +68,10 @@ describe( 'useNoRecursiveRenders', () => { it( 'allows equal but sibling blocks to render', () => { const { container } = render( - - - - + + + + ); expect( container.querySelectorAll( '.wp-block__reusable-block' ) @@ -68,9 +83,9 @@ describe( 'useNoRecursiveRenders', () => { it( 'prevents a block from rendering itself', () => { const { container } = render( - - - + + + ); expect( container.querySelectorAll( '.wp-block__reusable-block' ) @@ -80,11 +95,28 @@ describe( 'useNoRecursiveRenders', () => { ).toHaveLength( 1 ); } ); + it( 'prevents a block from rendering itself only when the same block type', () => { + const { container } = render( + + + + ); + expect( + container.querySelectorAll( '.wp-block__reusable-block' ) + ).toHaveLength( 1 ); + expect( + container.querySelectorAll( '.wp-block__another-block' ) + ).toHaveLength( 1 ); + expect( + container.querySelectorAll( '.wp-block__another-block--halted' ) + ).toHaveLength( 1 ); + } ); + it( 'prevents mutual recursion between two blocks', () => { const { container } = render( - - - + + + ); expect( container.querySelectorAll( '.wp-block__reusable-block' ) diff --git a/packages/block-library/src/block/index.php b/packages/block-library/src/block/index.php index 1cf289e5342bf..3613680e9e515 100644 --- a/packages/block-library/src/block/index.php +++ b/packages/block-library/src/block/index.php @@ -29,7 +29,7 @@ function render_block_core_block( $attributes ) { trigger_error( sprintf( // translators: %s is the user-provided title of the reusable block. - __( 'Could not render Reusable Block %s: blocks cannot be rendered inside themselves.' ), + __( 'Could not render Reusable Block %s. Block cannot be rendered inside itself.' ), $reusable_block->post_title ), E_USER_WARNING diff --git a/packages/block-library/src/template-part/edit/index.js b/packages/block-library/src/template-part/edit/index.js index 1bac85ac62e3f..ab9b9a76c6ea7 100644 --- a/packages/block-library/src/template-part/edit/index.js +++ b/packages/block-library/src/template-part/edit/index.js @@ -5,6 +5,7 @@ import { useSelect } from '@wordpress/data'; import { BlockControls, useBlockProps, + __experimentalUseNoRecursiveRenders as useNoRecursiveRenders, Warning, store as blockEditorStore, } from '@wordpress/block-editor'; @@ -33,6 +34,10 @@ export default function TemplatePartEdit( { } ) { const templatePartId = theme && slug ? theme + '//' + slug : null; + const [ hasAlreadyRendered, RecursionProvider ] = useNoRecursiveRenders( + templatePartId + ); + // Set the postId block attribute if it did not exist, // but wait until the inner blocks have loaded to allow // new edits to trigger this. @@ -90,8 +95,18 @@ export default function TemplatePartEdit( { ); } + if ( isEntityAvailable && hasAlreadyRendered ) { + return ( + + + { __( 'Block cannot be rendered inside itself.' ) } + + + ); + } + return ( - <> + } - + ); } diff --git a/packages/block-library/src/template-part/index.php b/packages/block-library/src/template-part/index.php index ff88b452587b0..df64764e2b275 100644 --- a/packages/block-library/src/template-part/index.php +++ b/packages/block-library/src/template-part/index.php @@ -13,14 +13,23 @@ * @return string The render. */ function render_block_core_template_part( $attributes ) { - $content = null; - $area = WP_TEMPLATE_PART_AREA_UNCATEGORIZED; + static $seen_ids = array(); + + $template_part_id = null; + $content = null; + $area = WP_TEMPLATE_PART_AREA_UNCATEGORIZED; if ( ! empty( $attributes['postId'] ) && get_post_status( $attributes['postId'] ) ) { + $template_part_id = $attributes['postId']; // If we have a post ID and the post exists, which means this template part // is user-customized, render the corresponding post content. $content = get_post( $attributes['postId'] )->post_content; - } elseif ( isset( $attributes['theme'] ) && wp_get_theme()->get_stylesheet() === $attributes['theme'] ) { + } elseif ( + isset( $attributes['slug'] ) && + isset( $attributes['theme'] ) && + wp_get_theme()->get_stylesheet() === $attributes['theme'] + ) { + $template_part_id = $attributes['theme'] . '//' . $attributes['slug']; $template_part_query = new WP_Query( array( 'post_type' => 'wp_template_part', @@ -56,12 +65,36 @@ function render_block_core_template_part( $attributes ) { } } - if ( is_null( $content ) ) { - return 'Template Part Not Found'; + if ( ! $template_part_id || is_null( $content ) ) { + return __( 'Template Part not found.' ); + } + + if ( isset( $seen_ids[ $template_part_id ] ) ) { + if ( ! is_admin() ) { + trigger_error( + sprintf( + // translators: %s are the block attributes. + __( 'Could not render Template Part block with the attributes: %s. Block cannot be rendered inside itself.' ), + wp_json_encode( $attributes ) + ), + E_USER_WARNING + ); + } + + // WP_DEBUG_DISPLAY must only be honored when WP_DEBUG. This precedent + // is set in `wp_debug_mode()`. + $is_debug = defined( 'WP_DEBUG' ) && WP_DEBUG && + defined( 'WP_DEBUG_DISPLAY' ) && WP_DEBUG_DISPLAY; + return $is_debug ? + // translators: Visible only in the front end, this warning takes the place of a faulty block. + __( '[block rendering halted]' ) : + ''; } // Run through the actions that are typically taken on the_content. - $content = do_blocks( $content ); + $seen_ids[ $template_part_id ] = true; + $content = do_blocks( $content ); + unset( $seen_ids[ $template_part_id ] ); $content = wptexturize( $content ); $content = convert_smilies( $content ); $content = wpautop( $content );