Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/wp-includes/class-wp-block-parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,31 @@ class WP_Block_Parser {
* return an error on invalid inputs.
*
* @since 5.0.0
* @since 7.2.0 Triggers a notice when the document is not a string, and returns
* an empty array unless the document is a stringable object.
*
* @param string $document Input document being parsed.
* @return array[]
*/
public function parse( $document ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don’t know if we should expect Stringable objects here, but it might help to add a check for this

if ( is_object( $document ) && method_exists( $document, '__toString' ) ) {
	_doing_it_wrong( ..., __( 'Convert to string before calling `parse()`' ), ... );
	$document = (string) $document;
}

thoughts? perhaps see if you can find evidence of any code in Core or int he plugin directory that passes non-string-but-Stringable values into parse_blocks()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks, @dmsnell. I didn't find anyone passing a Stringable, but I'd still add your check, because this PR changes what happens if someone does. A Stringable document parses normally on 7.1 and trunk, since PHP converts it to a string. With the PR as it stands, it returns an empty array.

What I checked:

  • Core: I found no parse_blocks() caller that passes an object. They pass post content, template or pattern content, or markup strings. do_blocks() passes on whatever the_content receives, so a plugin could still reach it that way.
  • Plugin directory: I searched it with Veloria and found about 4,800 lines that look like parse_blocks() calls. None pass $this, new ..., Str::... or str(...), or a variable named like an HTML processor. do_blocks() had no matches for those patterns either, and neither did the theme directory.
  • Limits: this is a text search. Most of those calls pass a variable or property, and I can't tell the runtime type from the line. So this shows no obvious use, not that there is none.

Done in afa82b1: a Stringable document now triggers the notice and is converted and parsed. I kept is_object() with method_exists() as you wrote it, because the minimum PHP version is below 8.0 and the template loader and patterns registry use the same check.

Also, to close the question from my earlier comment: I've now checked 7.1, and 123 comes back as a freeform block there too, the same as trunk.

if ( is_object( $document ) && method_exists( $document, '__toString' ) ) {
_doing_it_wrong(
__METHOD__,
__( 'Convert the document to a string before calling parse().' ),
'7.2.0'
);
$document = (string) $document;
}

if ( ! is_string( $document ) ) {
_doing_it_wrong(
__METHOD__,
__( 'The document must be a string.' ),
'7.2.0'
);
return array();
}

$this->document = $document;
$this->offset = 0;
$this->output = array();
Expand Down
62 changes: 62 additions & 0 deletions tests/phpunit/tests/blocks/wpBlockParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,68 @@ public function test_parses_basic_empty_block() {
);
}

/**
* Ensures that a document that is not a string returns an empty array and triggers a notice.
*
* @ticket 66138
*
* @covers ::parse
*
* @dataProvider data_non_string_documents
*
* @expectedIncorrectUsage WP_Block_Parser::parse
*
* @param mixed $document Document that is not a string.
*/
public function test_returns_empty_array_for_non_string_document( $document ) {
$p = new WP_Block_Parser();

$this->assertSame( array(), $p->parse( $document ) );
}

/**
* Data provider.
*
* @return array[]
*/
public static function data_non_string_documents() {
return array(
'null' => array( null ),
'false' => array( false ),
'integer' => array( 123 ),
'array' => array( array( 'content' ) ),
'object' => array( new stdClass() ),
);
}

/**
* Ensures that a stringable object is parsed as a string and triggers a notice.
*
* @ticket 66138
*
* @covers ::parse
*
* @expectedIncorrectUsage WP_Block_Parser::parse
*/
public function test_parses_stringable_document_as_string() {
$markup = '<!-- wp:paragraph --><p>Hello</p><!-- /wp:paragraph -->';
$document = new class( $markup ) {
private $markup;

public function __construct( $markup ) {
$this->markup = $markup;
}

public function __toString() {
return $this->markup;
}
};

$p = new WP_Block_Parser();

$this->assertSame( $p->parse( $markup ), $p->parse( $document ) );
}

/**
* Ensures that implicitly closing a final open block does not repeat content parsed before it.
*
Expand Down
Loading