REST API: regression after introducing changes how block renderer endpoint is now defined

[manooweb]

Hello,

We developed a dynamic block with only boolean block attributes.
For this development we used WordPress 5.4.2 and everything works fine.

We are testing with the future version of WordPress 5.5 ( WordPress 5.5-beta1-48410 ) and we noticed a regression because sanitization of block attributes doesn't work anymore espacially for boolean attributes which is important to get the right value in PHP (true or false) and not a string.

Indeed there has been changes in the REST API block renderer endpoint definition introduced by this changeset [48069] to fix this ticket #48079.

As we can see in the endpoint schema we lost the 'properties' attributes which is used before to set the block attributes.

Everything is ok during the attributes validation which passes through the 'validate_callback' defined in the endpoint.

However during the REST API process we also pass through a sanitization step here ​https://github.com/WordPress/WordPress/blob/master/wp-includes/rest-api/class-wp-rest-server.php#L965

and then here for block attributes
​https://github.com/WordPress/WordPress/blob/master/wp-includes/rest-api.php#L1864

We never pass through the condition because the 'properties' array is never defined at this point as it should be.

As every dynamic blocks need their own attributes, I don't understand how to set correctly this 'properties' array to get the values correctly sanitized.

Of course we found a workaround on our side directly in our 'render_callback' of our dynamic block function but I think it is not the best and right solution.

If you need further information, let me know.

Regards

[TimothyBlynJacobs]

Hi @manooweb,

Thanks for the ticket! That's a great catch. Do you want to work on a patch for it? We'd essentially want to duplicate our validate_callback that is set, but at the end call rest_sanitize_value_from_schema instead of the validate function.

[manooweb]

Hi Timothy,

You're welcome

Of course, I can try.
What do you mean excatly by duplicate

Create a sanitize_callback entry which duplicates the code of validate_callback except the call to rest_validate_value_from_schema replaced by a call to rest_sanitize_value_from_schema ?

[manooweb]

Timothy, I attached the patch. I tested manually on my WordPress environment and solve the issue on our dynamic block.

Do I also have to work on unit tests ?

[manooweb]

Add sanitize_callback in block renderer endpoint

[TimothyBlynJacobs]

That looks great, thanks!

Do I also have to work on unit tests ?

You can work on the tests if you want to, but I can add them if you don't. If you are interested, this might be a helpful guide: ​https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/

[manooweb]

Add unit test

[manooweb]

Hi Timothy,
I've just added a new patch containing the fix and thus a unit test.

Have a nice week-end 😉

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

[TimothyBlynJacobs]

Thanks for the tests @manooweb! Committing with some minor changes to the tests to make them more explicit.

[TimothyBlynJacobs]

In 48437:

REST API: Sanitize block renderer attributes.

In [48069] the Block Renderer was changed to register a single route for all dynamic blocks. Validation was dynamically applied based on the requested block, but sanitization was not. This commit adds the same sanitization back to the block attributes.

Props manooweb.
Fixes #50620. See #48079.

so what about changes how to do that