WordPress.org

Make WordPress Core

Opened 2 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#49680 closed enhancement (fixed)

Consider also allowing POST requests for block ServerSideRender endpoint

Reported by: ryankienstra Owned by: TimothyBlynJacobs
Milestone: 5.5 Priority: normal
Severity: normal Version: 5.0
Component: REST API Keywords: has-patch has-unit-tests
Focuses: rest-api Cc:

Description

The ServerSideRender component uses GET requests to send the attributes to its endpoint.

But sometimes the attributes are too big to send in the URL of the GET request, and cause an error:

https://github.com/WordPress/gutenberg/issues/19935
https://github.com/WordPress/gutenberg/issues/16396#issuecomment-508709339
https://wordpress.org/support/topic/error-loading-block-the-response-is-not-a-valid-json-response-2/

There have been suggestions in those issues to allow using a POST request for <ServerSideRender>. This would allow sending a much bigger attributes object to the endpoint, and prevent this error.

As blocks become more ubiquitous, like with Full Site Editing, I think this would help.

The patch for this would probably be like:

diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-block-renderer-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-block-renderer-controller.php
index 94d349a1ba..bc1dbd3e83 100644
--- a/src/wp-includes/rest-api/endpoints/class-wp-rest-block-renderer-controller.php
+++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-block-renderer-controller.php
@@ -52,7 +52,7 @@ class WP_REST_Block_Renderer_Controller extends WP_REST_Controller {
                                                ),
                                        ),
                                        array(
-                                               'methods'             => WP_REST_Server::READABLE,
+                                               'methods'             => array( WP_REST_Server::READABLE, WP_REST_Server::CREATABLE ),
                                                'callback'            => array( $this, 'get_item' ),
                                                'permission_callback' => array( $this, 'get_item_permissions_check' ),
                                                'args'                => array(
@@ -148,7 +148,8 @@ class WP_REST_Block_Renderer_Controller extends WP_REST_Controller {
                        );
                }
 
-               $attributes = $request->get_param( 'attributes' );
+               // In a POST request, the attributes appear as JSON in the request body.
+               $attributes = WP_REST_Server::CREATABLE === $request->get_method() ? json_decode( $request->get_body(), true ) : $request->get_param( 'attributes' );
 
                // Create an array representation simulating the output of parse_blocks.

Thanks!

Change History (17)

This ticket was mentioned in PR #196 on WordPress/wordpress-develop by kienstra.


2 months ago

<!--
Hi there! Thanks for contributing to WordPress!

Pull Requests in this GitHub repository must be linked to a ticket in the WordPress Core Trac instance (https://core.trac.wordpress.org), and are only used for code review. No pull requests will be merged on GitHub.

See the WordPress Handbook page on using PRs for Code Review more information: https://make.wordpress.org/core/handbook/contribute/git/github-pull-requests-for-code-review/

If this is your first time contributing, you may also find reviewing these guides first to be helpful:

-->

This allows a block's <ServerSideRender> component to make a POST request, in case the attributes are too big for a GET request.

There have been several cases where sending the attributes in the GET request URL causes an error:

https://github.com/WordPress/gutenberg/issues/19935
https://github.com/WordPress/gutenberg/issues/16396#issuecomment-508709339
https://wordpress.org/support/topic/error-loading-block-the-response-is-not-a-valid-json-response-2/

This follows the suggestion in 2 of those issues to allow using POST for the block endpoint, in addition to GET.

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

#2 @prbot
2 months ago

kienstra commented on PR #196:

It doesn't look like the [Travis failures](https://github.com/WordPress/wordpress-develop/runs/526164894) are related to this PR, though maybe I missed something.

#3 @prbot
2 months ago

kienstra commented on PR #196:

https://github.com/WordPress/gutenberg/pull/21068 is what actually uses this PR's addition of POST to the allowed requests.

#4 @TimothyBlynJacobs
2 months ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.5

Hi @ryankienstra!

Thanks for opening a ticket! I'm milestoning this for 5.5 to evaluate along with other block renderer changes in #48079.

#5 @TimothyBlynJacobs
2 months ago

  • Version changed from trunk to 5.0

#6 @ryankienstra
2 months ago

Thanks, I really appreciate it!

#7 @TimothyBlynJacobs
6 weeks ago

Hi @ryankienstra,

The idea of passing just the attributes as the JSON/POST body directly is interesting. But it isn't consistent with how the REST API generally handles parameters. WP_REST_Request::get_param will traverse all of the parameter locations looking for an attribute.

Besides the inconsistencies, this would cause breakage if a block had a post_id attribute for example. Since it would fetch the value from the attributes, instead of the URL.

Instead, we should just expand the accepted methods to include WP_REST_Server::CREATABLE. And then attributes can be passed in the body nested under attributes.

#8 @ryankienstra
5 weeks ago

Hi @TimothyBlynJacobs,
Thanks a lot for your suggestion, sorry for the delay.

Is this what you had in mind? https://github.com/WordPress/wordpress-develop/pull/196/commits/fad2b162985a82fe85dc9d5a2e0d30a008e0aac0

So the only change is adding WP_REST_Server::CREATABLE, as you mentioned.

https://github.com/WordPress/wordpress-develop/pull/196/files

Thanks, Timothy!

#9 @TimothyBlynJacobs
5 weeks ago

Yep @ryankienstra, that's it. No worries!

I believe the test failure is because you are not setting a application/json content-type header. You could also test using set_body_params which would be a bit simpler.

#10 @ryankienstra
5 weeks ago

Thanks for your suggestions, I'm working on fixing the failed test.

#11 @ryankienstra
5 weeks ago

Hi @TimothyBlynJacobs,
Thanks for helping with this.

The test is passing in Travis, is it alright as it is? https://github.com/WordPress/wordpress-develop/pull/196/files

I couldn't get it to pass locally with set_body_params().

Thanks!

#12 @TimothyBlynJacobs
5 weeks ago

No problem, yep, that looks good!

#13 @TimothyBlynJacobs
5 weeks ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

#14 @ryankienstra
5 weeks ago

Thanks for all of your help!

#15 @TimothyBlynJacobs
4 weeks ago

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

In 47756:

REST API: Accept POST requests in the block renderer endpoint.

Rendering a block is idempotent, so a GET is the most natural request method. However, the maximum length of URLs prevented blocks with large attributes from being rendered.

Props ryankienstra.
Fixes #49680.

#17 @prbot
4 weeks ago

kienstra commented on PR #196:

Nice, thanks so much!

Note: See TracTickets for help on using tickets.