Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#52364 closed task (blessed) (fixed)

Reusable blocks: Add tests for recursion, subsequent instances

Reported by: bernhard-reiter's profile Bernhard Reiter Owned by: peterwilsoncc's profile peterwilsoncc
Milestone: 5.7 Priority: normal
Severity: normal Version: 5.7
Component: Editor Keywords: has-patch has-unit-tests commit
Focuses: Cc:

Description

As of Gutenberg 9.8.1, it's possible to insert a reusable block into itself. This creates an infinite recursion, triggering a server error (500), and possibly crashing the editor. (We seem to encounter the latter behavior more frequently, per the recent [reusable blocks refactor]https://github.com/WordPress/gutenberg/pull/27887.)

This has been recently fixed in Gutenberg (gb#28405 and gb#28428). (It was then amended by gb#28461 to continue to allow rendering of multiple non-nested reusable blocks in a post.)

Reusable blocks are covered by PHP unit tests found in `tests/phpunit/tests/blocks/render-reusable.php`.

We should add tests to:

  • Ensure recursions are prevented, and that
  • Rendering of subsequent (non-nested) reusable blocks still works.

I have a diff ready that I'll file as a PR against the wordpress-develop repo shortly.

(On a sidenote, in order to prevent regressions in the future, we should consider running Core's unit tests against PRs in the Gutenberg repo, see gb#26418).

Attachments (1)

52364.diff (1.7 KB) - added by peterwilsoncc 4 years ago.

Download all attachments as: .zip

Change History (7)

This ticket was mentioned in PR #920 on WordPress/wordpress-develop by ockham.


4 years ago
#1

  • Keywords has-patch has-unit-tests added

As of Gutenberg 9.8.1, it's possible to insert a reusable block into itself. This creates an infinite recursion, triggering a server error (500), and possibly crashing the editor. (We seem to encounter the latter behavior more frequently, per the recent reusable blocks refactor.)

This has been recently fixed by @mcsf in Gutenberg (https://github.com/WordPress/gutenberg/pull/28405 and https://github.com/WordPress/gutenberg/pull/28428). (It was then amended by https://github.com/WordPress/gutenberg/pull/28461 to continue to allow rendering of multiple non-nested reusable blocks in a post.)

Reusable blocks are covered by PHP unit tests found in `tests/phpunit/tests/blocks/render-reusable.php`.

This PR adds tests to:

  • Ensure recursions are prevented, and that
  • Rendering of subsequent (non-nested) reusable blocks still works.

Please note that these newly added tests will currently fail (e.g. in CI), unless the aforementioned Gutenberg PRs are carried over to WordPress**

(On a sidenote, in order to prevent regressions in the future, we should consider running Core's unit tests against PRs in the Gutenberg repo, see https://github.com/WordPress/gutenberg/issues/26418).

Trac ticket: https://core.trac.wordpress.org/ticket/52364

cc/ @mcsf @gziolo @sirreal

#2 @SergeyBiryukov
4 years ago

  • Component changed from General to Editor

#3 @SergeyBiryukov
4 years ago

  • Milestone changed from Awaiting Review to 5.7
  • Type changed from enhancement to task (blessed)

@peterwilsoncc
4 years ago

#4 @peterwilsoncc
4 years ago

  • Keywords commit added

In 52364.diff :

  • bring the PR patch in to trac
  • As many globals are reset between tests, I've called $block_type->render() twice in test_render_subsequent() to make sure any relevant globals haven't been reset since the previous test.
  • All post objects are reset to their original state at the end of each test, I've removed the manual call to wp_update_post() at the end of test_recursive_render_warning() as adds unnecessary database writes to the test suite.

This looks ready to me, marking as such.

Related #51802.

#5 @peterwilsoncc
4 years ago

  • Owner set to peterwilsoncc
  • Resolution set to fixed
  • Status changed from new to closed

In 50382:

Editor: Additional tests for reusable blocks.

Ensure that subsequent renders of a reusable block will render correctly and that recursively inserting a reusable block into itself does not cause an internal server (500) error.

Props bernhard-reiter, SergeyBiryukov.
Fixes #52364.

Note: See TracTickets for help on using tickets.