diff --git a/packages/block-library/src/pattern/edit.js b/packages/block-library/src/pattern/edit.js index e01fb37eb849e..7befaa9cb2dee 100644 --- a/packages/block-library/src/pattern/edit.js +++ b/packages/block-library/src/pattern/edit.js @@ -3,12 +3,19 @@ */ import { cloneBlock } from '@wordpress/blocks'; import { useSelect, useDispatch } from '@wordpress/data'; -import { useEffect } from '@wordpress/element'; +import { useState, useEffect } from '@wordpress/element'; import { + Warning, store as blockEditorStore, useBlockProps, } from '@wordpress/block-editor'; import { store as coreStore } from '@wordpress/core-data'; +import { __, sprintf } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import { useParsePatternDependencies } from './recursion-detector'; const PatternEdit = ( { attributes, clientId } ) => { const selectedPattern = useSelect( @@ -32,6 +39,9 @@ const PatternEdit = ( { attributes, clientId } ) => { const { getBlockRootClientId, getBlockEditingMode } = useSelect( blockEditorStore ); + const [ hasRecursionError, setHasRecursionError ] = useState( false ); + const parsePatternDependencies = useParsePatternDependencies(); + // Duplicated in packages/edit-site/src/components/start-template-options/index.js. function injectThemeAttributeInBlockTemplateContent( block ) { if ( @@ -64,7 +74,14 @@ const PatternEdit = ( { attributes, clientId } ) => { // This change won't be saved. // It will continue to pull from the pattern file unless changes are made to its respective template part. useEffect( () => { - if ( selectedPattern?.blocks ) { + if ( ! hasRecursionError && selectedPattern?.blocks ) { + try { + parsePatternDependencies( selectedPattern ); + } catch ( error ) { + setHasRecursionError( true ); + return; + } + // We batch updates to block list settings to avoid triggering cascading renders // for each container block included in a tree and optimize initial render. // Since the above uses microtasks, we need to use a microtask here as well, @@ -93,7 +110,8 @@ const PatternEdit = ( { attributes, clientId } ) => { } }, [ clientId, - selectedPattern?.blocks, + hasRecursionError, + selectedPattern, __unstableMarkNextChangeAsNotPersistent, replaceBlocks, getBlockEditingMode, @@ -103,6 +121,20 @@ const PatternEdit = ( { attributes, clientId } ) => { const props = useBlockProps(); + if ( hasRecursionError ) { + return ( +
+ + { sprintf( + // translators: A warning in which %s is the name of a pattern. + __( 'Pattern "%s" cannot be rendered inside itself.' ), + selectedPattern?.name + ) } + +
+ ); + } + return
; }; diff --git a/packages/block-library/src/pattern/index.php b/packages/block-library/src/pattern/index.php index 436452f685300..70c389e4ec8db 100644 --- a/packages/block-library/src/pattern/index.php +++ b/packages/block-library/src/pattern/index.php @@ -27,6 +27,8 @@ function register_block_core_pattern() { * @return string Returns the output of the pattern. */ function render_block_core_pattern( $attributes ) { + static $seen_refs = array(); + if ( empty( $attributes['slug'] ) ) { return ''; } @@ -38,6 +40,17 @@ function render_block_core_pattern( $attributes ) { return ''; } + if ( isset( $seen_refs[ $attributes['slug'] ] ) ) { + // WP_DEBUG_DISPLAY must only be honored when WP_DEBUG. This precedent + // is set in `wp_debug_mode()`. + $is_debug = WP_DEBUG && WP_DEBUG_DISPLAY; + + return $is_debug ? + // translators: Visible only in the front end, this warning takes the place of a faulty block. %s represents a pattern's slug. + sprintf( __( '[block rendering halted for pattern "%s"]' ), $slug ) : + ''; + } + $pattern = $registry->get_registered( $slug ); $content = $pattern['content']; @@ -48,11 +61,14 @@ function render_block_core_pattern( $attributes ) { $content = gutenberg_serialize_blocks( $blocks ); } + $seen_refs[ $attributes['slug'] ] = true; + $content = do_blocks( $content ); global $wp_embed; $content = $wp_embed->autoembed( $content ); + unset( $seen_refs[ $attributes['slug'] ] ); return $content; } diff --git a/packages/block-library/src/pattern/recursion-detector.js b/packages/block-library/src/pattern/recursion-detector.js new file mode 100644 index 0000000000000..f2e80087dd4b0 --- /dev/null +++ b/packages/block-library/src/pattern/recursion-detector.js @@ -0,0 +1,145 @@ +/** + * THIS MODULE IS INTENTIONALLY KEPT WITHIN THE PATTERN BLOCK'S SOURCE. + * + * This is because this approach for preventing infinite loops due to + * recursively rendering blocks is specific to the way that the `core/pattern` + * block behaves in the editor. Any other block types that deal with recursion + * SHOULD USE THE STANDARD METHOD for avoiding loops: + * + * @see https://github.com/WordPress/gutenberg/pull/31455 + * @see packages/block-editor/src/components/recursion-provider/README.md + */ + +/** + * WordPress dependencies + */ +import { useRegistry } from '@wordpress/data'; + +/** + * Naming is hard. + * + * @see useParsePatternDependencies + * + * @type {WeakMap} + */ +const cachedParsers = new WeakMap(); + +/** + * Hook used by PatternEdit to parse block patterns. It returns a function that + * takes a pattern and returns nothing but throws an error if the pattern is + * recursive. + * + * @example + * ```js + * const parsePatternDependencies = useParsePatternDependencies(); + * parsePatternDependencies( selectedPattern ); + * ``` + * + * @see parsePatternDependencies + * + * @return {Function} A function to parse block patterns. + */ +export function useParsePatternDependencies() { + const registry = useRegistry(); + + // Instead of caching maps, go straight to the point and cache bound + // functions. Each of those functions is bound to a different Map that will + // keep track of patterns in the context of the given registry. + if ( ! cachedParsers.has( registry ) ) { + const deps = new Map(); + cachedParsers.set( + registry, + parsePatternDependencies.bind( null, deps ) + ); + } + return cachedParsers.get( registry ); +} + +/** + * Parse a given pattern and traverse its contents to detect any subsequent + * patterns on which it may depend. Such occurrences will be added to an + * internal dependency graph. If a circular dependency is detected, an + * error will be thrown. + * + * EXPORTED FOR TESTING PURPOSES ONLY. + * + * @param {Map>} deps Map of pattern dependencies. + * @param {Object} pattern Pattern. + * @param {string} pattern.name Pattern name. + * @param {Array} pattern.blocks Pattern's block list. + * + * @throws {Error} If a circular dependency is detected. + */ +export function parsePatternDependencies( deps, { name, blocks } ) { + const queue = [ ...blocks ]; + while ( queue.length ) { + const block = queue.shift(); + for ( const innerBlock of block.innerBlocks ?? [] ) { + queue.unshift( innerBlock ); + } + if ( block.name === 'core/pattern' ) { + registerDependency( deps, name, block.attributes.slug ); + } + } +} + +/** + * Declare that pattern `a` depends on pattern `b`. If a circular + * dependency is detected, an error will be thrown. + * + * EXPORTED FOR TESTING PURPOSES ONLY. + * + * @param {Map>} deps Map of pattern dependencies. + * @param {string} a Slug for pattern A. + * @param {string} b Slug for pattern B. + * + * @throws {Error} If a circular dependency is detected. + */ +export function registerDependency( deps, a, b ) { + if ( ! deps.has( a ) ) { + deps.set( a, new Set() ); + } + deps.get( a ).add( b ); + if ( hasCycle( deps, a ) ) { + throw new TypeError( + `Pattern ${ a } has a circular dependency and cannot be rendered.` + ); + } +} + +/** + * Determine if a given pattern has circular dependencies on other patterns. + * This will be determined by running a depth-first search on the current state + * of the graph represented by `patternDependencies`. + * + * @param {Map>} deps Map of pattern dependencies. + * @param {string} slug Pattern slug. + * @param {Set} [visitedNodes] Set to track visited nodes in the graph. + * @param {Set} [currentPath] Set to track and backtrack graph paths. + * @return {boolean} Whether any cycle was found. + */ +function hasCycle( + deps, + slug, + visitedNodes = new Set(), + currentPath = new Set() +) { + visitedNodes.add( slug ); + currentPath.add( slug ); + + const dependencies = deps.get( slug ) ?? new Set(); + + for ( const dependency of dependencies ) { + if ( ! visitedNodes.has( dependency ) ) { + if ( hasCycle( deps, dependency, visitedNodes, currentPath ) ) { + return true; + } + } else if ( currentPath.has( dependency ) ) { + return true; + } + } + + // Remove the current node from the current path when backtracking + currentPath.delete( slug ); + return false; +} diff --git a/packages/block-library/src/pattern/test/index.js b/packages/block-library/src/pattern/test/index.js new file mode 100644 index 0000000000000..0f682f6cce67b --- /dev/null +++ b/packages/block-library/src/pattern/test/index.js @@ -0,0 +1,74 @@ +/** + * Internal dependencies + */ +import { + parsePatternDependencies, + registerDependency, +} from '../recursion-detector'; + +describe( 'core/pattern', () => { + const deps = new Map(); + + beforeEach( () => { + deps.clear(); + } ); + + describe( 'parsePatternDependencies', () => { + it( "is silent for patterns that don't require other patterns", () => { + const pattern = { + name: 'test/benign-pattern', + blocks: [ { name: 'core/paragraph' } ], + }; + expect( () => { + parsePatternDependencies( deps, pattern ); + } ).not.toThrow(); + } ); + it( 'catches self-referencing patterns', () => { + const pattern = { + name: 'test/evil-pattern', + blocks: [ { name: 'core/pattern', slug: 'test/evil-pattern' } ], + }; + expect( () => { + parsePatternDependencies( deps, pattern ); + } ).toThrow(); + } ); + } ); + + describe( 'registerDependency', () => { + it( 'is silent for patterns with no circular dependencies', () => { + expect( () => { + registerDependency( deps, 'a', 'b' ); + } ).not.toThrow(); + } ); + it( 'catches self-referencing patterns', () => { + expect( () => { + registerDependency( deps, 'a', 'a' ); + } ).toThrow(); + } ); + it( 'catches mutually-referencing patterns', () => { + registerDependency( deps, 'a', 'b' ); + expect( () => { + registerDependency( deps, 'b', 'a' ); + } ).toThrow(); + } ); + it( 'catches longer cycles', () => { + registerDependency( deps, 'a', 'b' ); + registerDependency( deps, 'b', 'c' ); + registerDependency( deps, 'b', 'd' ); + expect( () => { + registerDependency( deps, 'd', 'a' ); + } ).toThrow(); + } ); + it( 'catches any pattern depending on a tainted one', () => { + registerDependency( deps, 'a', 'b' ); + registerDependency( deps, 'b', 'c' ); + registerDependency( deps, 'b', 'd' ); + expect( () => { + registerDependency( deps, 'd', 'a' ); + } ).toThrow(); + expect( () => { + registerDependency( deps, 'e', 'd' ); + } ).toThrow(); + } ); + } ); +} ); diff --git a/packages/e2e-tests/plugins/pattern-recursion.php b/packages/e2e-tests/plugins/pattern-recursion.php new file mode 100644 index 0000000000000..eb3a2051551a2 --- /dev/null +++ b/packages/e2e-tests/plugins/pattern-recursion.php @@ -0,0 +1,22 @@ + 'Evil recursive', + 'description' => 'Evil recursive', + 'content' => '

Hello

', + ) + ); + } +); diff --git a/test/e2e/specs/editor/plugins/pattern-recursion.spec.js b/test/e2e/specs/editor/plugins/pattern-recursion.spec.js new file mode 100644 index 0000000000000..72498bcdf9914 --- /dev/null +++ b/test/e2e/specs/editor/plugins/pattern-recursion.spec.js @@ -0,0 +1,35 @@ +/** + * WordPress dependencies + */ +const { test, expect } = require( '@wordpress/e2e-test-utils-playwright' ); + +test.describe( 'Preventing Pattern Recursion', () => { + test.beforeAll( async ( { requestUtils } ) => { + await requestUtils.activatePlugin( + 'gutenberg-test-protection-against-recursive-patterns' + ); + } ); + + test.beforeEach( async ( { admin } ) => { + await admin.createNewPost(); + } ); + + test.afterAll( async ( { requestUtils } ) => { + await requestUtils.deactivatePlugin( + 'gutenberg-test-protection-against-recursive-patterns' + ); + } ); + + test( 'prevents infinite loops due to recursive patterns', async ( { + editor, + } ) => { + await editor.insertBlock( { + name: 'core/pattern', + attributes: { slug: 'evil/recursive' }, + } ); + const warning = editor.canvas.getByText( + 'Pattern "evil/recursive" cannot be rendered inside itself' + ); + await expect( warning ).toBeVisible(); + } ); +} );