-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
+327
−3
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
014a684
Prevent infinite loops due to recursive patterns
mcsf 62e3b0b
Rename, add private method, export singleton
mcsf 08546e7
Rename module to ./recursion-detector
mcsf 2b7a974
Improve naming of variables
mcsf 109fcd8
(m) naming
mcsf 06edbd2
Patterns: prevent server-side infinite recursion
mcsf 559623e
Improve messaging by mentioning offending pattern
mcsf ca82ca5
Add tests for cycle detection
mcsf 16a8fec
Add disclaimer about detection method
mcsf 940e68b
Add usePatternRecursionDetector with registry-based WeakMap
mcsf 18c8edc
Update cache name, add comments
mcsf d09b7c9
Add E2E guarding against pattern recursion
mcsf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
145 changes: 145 additions & 0 deletions
145
packages/block-library/src/pattern/recursion-detector.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} ); | ||
} ); | ||
} ); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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"} /-->', | ||
) | ||
); | ||
} | ||
); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} ); | ||
} ); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.