Skip to content

Commit

Permalink
Patterns (unsynced): prevent infinite loops due to recursive patterns (
Browse files Browse the repository at this point in the history
…#56511)

* Prevent infinite loops due to recursive patterns

* Add tests for cycle detection

* Add disclaimer about detection method

* Add E2E guarding against pattern recursion
  • Loading branch information
mcsf authored and artemiomorales committed Jan 4, 2024
1 parent f0fd026 commit c78544c
Show file tree
Hide file tree
Showing 6 changed files with 327 additions and 3 deletions.
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();
} );
} );

0 comments on commit c78544c

Please sign in to comment.