The rest_prepare_attachment filter runs twice during REST request

[chrisl27]

Hi,

For each /wp/v2/media call the rest_prepare_attachment filter is being run twice. Once by WP_REST_Posts_Controller and then a second time by WP_REST_Attachments_Controller.

This is due to WP_REST_Attachments_Controller::prepare_item_for_response calling its class parent method, which calls apply_filters on "rest_prepare_{$this->post_type}"

This may cause issues with code that expects the attachment to only be prepared once, particularly where the response payload is different between the two filter callbacks (eg doesn't include media sizes the first time).

[chrisl27]

Patch for this issue, with test

[chrisl27]

Patch has test that will fail without the code change (indicating the filter has run twice)

[chrisl27]

This patch is a simple solution to the attachment problem that doesn't help if you otherwise subclassed WP_REST_Posts_Controller for your own post type and wanted to include a filter. I think that's unlikely to happen outside of core though.

[TimothyBlynJacobs]

Thanks for the ticket @chrisl27!

In core this is just the attachments controller, so we could do the hardcoded check for the attachment post type, but this is a tricky issue since it'd apply whenever a plugin tries to override prepare_item_for_response.

I wonder if instead we should refactor the attachments controller to use the filter instead of overriding the method, and encourage plugins to use the filter.

[chrisl27]

Replying to TimothyBlynJacobs:

Thanks for the ticket @chrisl27!

In core this is just the attachments controller, so we could do the hardcoded check for the attachment post type, but this is a tricky issue since it'd apply whenever a plugin tries to override prepare_item_for_response.

I wonder if instead we should refactor the attachments controller to use the filter instead of overriding the method, and encourage plugins to use the filter.

Yup, agreed it's not ideal as it makes the problem more obscure for non-attachments.

I don't think there is anywhere else in the API code where the code uses a filter the API itself created so that would be unusual too.

How about changing the method signature of prepare_item_for_response to something like

public function prepare_item_for_response( $post, $request, $suppress_filters = false );

and call parent::prepare_item_for_response($post, $request, true); from the attachments subclass.

I think this could work because the attachments controller can actually look at the same flag to see if *it* should suppress_filters on the response as well, enabling a further subclass of the attachments controller to suppress filters for both its parents.

It also has the advantage that the function signature implies there's something going on with filters that would be clearer to plugin or core authors. A boolean parameter may not be ideal for future changes, perhaps a wp_parse_args array param as the third argument if we go this way?

[chrisl27]

Although that would be a breaking change for any existing subclasses that override that method. If that's a concern then maybe a new protected variable could control this.