WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 4 months ago

#36271 closed task (blessed) (fixed)

Trigger _doing_it_wrong if wp_send_json is used on a REST API request

Reported by: rmccue Owned by: spacedmonkey
Milestone: 5.5 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch
Focuses: Cc:

Description

We should heavily discourage users from using wp_send_json (& co.) on REST API requests. Two changes I want to make:

  1. If _doing_it_wrong is triggered on a REST request, we should add a header ala the _deprecated_function headers.
  2. If wp_send_json is called on a REST request, we should trigger a _doing_it_wrong and point people towards WP_REST_Response and WP_Error

Change History (14)

#1 @rachelbaker
4 years ago

  • Keywords has-unit-tests added
  • Milestone changed from Awaiting Review to Future Release

Moved into Future Release, needs a patch.

#2 @TimothyBlynJacobs
5 months ago

  • Keywords good-first-bug added
  • Milestone set to 5.5
  • Version set to 4.4

I still see this occurring in the wild. Milestoning for 5.5.

#3 @johnbillion
5 months ago

  • Keywords has-unit-tests good-first-bug removed

This ticket was mentioned in PR #325 on WordPress/wordpress-develop by TimothyBJacobs.


5 months ago

  • Keywords has-patch added; needs-patch removed

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


4 months ago

#6 @TimothyBlynJacobs
4 months ago

@SergeyBiryukov could you take a look at this one if you have a chance?

#7 @TimothyBlynJacobs
4 months ago

  • Owner set to spacedmonkey
  • Status changed from new to reviewing

#8 @whyisjake
4 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 48361:

REST API: Trigger _doing_it_wrong() if wp_send_json() is used on a REST API request

In addition to triggering the _doing_it_wrong() logging, also adds a X-WP-DoingItWrong header.

Fixes #36271.

Props rmccue, TimothyBlynJacobs.

#9 @SergeyBiryukov
4 months ago

In 48367:

REST API: Correct the check for $version argument in rest_handle_doing_it_wrong().

Move WP_REST_Response and WP_Error class names out of the translatable string.

Follow-up to [48327], [48361].

See #36271.

#10 @spacedmonkey
4 months ago

I would not have used

if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) {

here. I would have used.

if ( wp_is_json_request() ) {

CC @whyisjake @SergeyBiryukov

#11 @SergeyBiryukov
4 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@TimothyBlynJacobs Could you double-check comment:10?

At a glance, checking REST_REQUEST seems correct to me, as the message is specific to REST API, but I'm not deeply familiar with the history here.

#12 @SergeyBiryukov
4 months ago

  • Type changed from enhancement to task (blessed)

#13 @TimothyBlynJacobs
4 months ago

The constant comes from #50318.

#14 @TimothyBlynJacobs
4 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

Reclosing this, if we want to adjust the constant check it should be done in that ticket.

Note: See TracTickets for help on using tickets.