Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patterns (unsynced): prevent infinite loops due to recursive patterns #56511

Merged
merged 12 commits into from
Dec 15, 2023
38 changes: 35 additions & 3 deletions packages/block-library/src/pattern/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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 (
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -93,7 +110,8 @@ const PatternEdit = ( { attributes, clientId } ) => {
}
}, [
clientId,
selectedPattern?.blocks,
hasRecursionError,
selectedPattern,
__unstableMarkNextChangeAsNotPersistent,
replaceBlocks,
getBlockEditingMode,
Expand All @@ -103,6 +121,20 @@ const PatternEdit = ( { attributes, clientId } ) => {

const props = useBlockProps();

if ( hasRecursionError ) {
return (
<div { ...props }>
<Warning>
{ sprintf(
// translators: A warning in which %s is the name of a pattern.
__( 'Pattern "%s" cannot be rendered inside itself.' ),
selectedPattern?.name
) }
</Warning>
</div>
);
}

return <div { ...props } />;
};

Expand Down
16 changes: 16 additions & 0 deletions packages/block-library/src/pattern/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 '';
}
Expand All @@ -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'];

Expand All @@ -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;
}

Expand Down
145 changes: 145 additions & 0 deletions packages/block-library/src/pattern/recursion-detector.js
Original file line number Diff line number Diff line change
@@ -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<Object, Function>}
*/
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<string, Set<string>>} 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<string, Set<string>>} 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<string, Set<string>>} deps Map of pattern dependencies.
* @param {string} slug Pattern slug.
* @param {Set<string>} [visitedNodes] Set to track visited nodes in the graph.
* @param {Set<string>} [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;
}
74 changes: 74 additions & 0 deletions packages/block-library/src/pattern/test/index.js
Original file line number Diff line number Diff line change
@@ -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();
} );
} );
} );
22 changes: 22 additions & 0 deletions packages/e2e-tests/plugins/pattern-recursion.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php
/**
* Plugin Name: Gutenberg Test Protection Against Recursive Patterns
* Plugin URI: https://github.com/WordPress/gutenberg
* Author: Gutenberg Team
*
* @package gutenberg-test-pattern-recursion
*/

add_filter(
'init',
function () {
register_block_pattern(
'evil/recursive',
array(
'title' => 'Evil recursive',
'description' => 'Evil recursive',
'content' => '<!-- wp:paragraph --><p>Hello</p><!-- /wp:paragraph --><!-- wp:pattern {"slug":"evil/recursive"} /-->',
)
);
}
);
35 changes: 35 additions & 0 deletions test/e2e/specs/editor/plugins/pattern-recursion.spec.js
Original file line number Diff line number Diff line change
@@ -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();
} );
} );
Copy link
Contributor

Choose a reason for hiding this comment

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

This reads well. Nice test.

Loading