WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 4 weeks ago

Last modified 4 weeks ago

#53056 closed defect (bug) (fixed)

REST API json_encode error returns 500 data->status in body(Correct) but 200 in actual status(Incorrect).

Reported by: lalitjalandhar Owned by: TimothyBlynJacobs
Milestone: 5.9 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: good-first-bug has-patch has-unit-tests dev-feedback
Focuses: rest-api Cc:

Description

<?php
add_action( 'rest_api_init', function() {
        register_rest_route(
                'v3',
                '/my-custom-route',
                array(
                        array(
                                'methods'             => \WP_REST_Server::READABLE,
                                'callback'            => function() {
                                        return new \WP_REST_Response(INF);
                                },
                                'permission_callback' => '__return_true',
                                'args'                => array(),
                        ),
                )
        );
});

Attachments (2)

nan.PNG (19.1 KB) - added by lalitjalandhar 8 months ago.
postman screen shot
53056.diff (2.4 KB) - added by hermpheus 7 months ago.

Download all attachments as: .zip

Change History (11)

@lalitjalandhar
8 months ago

postman screen shot

#1 @desrosj
7 months ago

  • Focuses rest-api added
  • Keywords needs-patch good-first-bug needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.8

Thanks for this one, @lalitjalandhar. And welcome to Trac!

It seems that there is a missing set_status() call here. Would you be interested in creating a patch?

It would also be nice to add a few tests to confirm the desired behavior after these changes.

This ticket was mentioned in PR #1264 on WordPress/wordpress-develop by hmfs.


7 months ago

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

Fixes issue in the REST API where a JSON encode failure of the response body does not result in a 500 error.

An automated test is included for this case, requiring an expansion of the test spy.

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

@hermpheus
7 months ago

#3 @hermpheus
7 months ago

Thank you @lalitjalandhar for the excellent example, and thank you @desrosj for the excellent suggestion.

I have a patch available that should address this issue. It has an automated test and also works for me in the browser and Postman. Please let me know if any changes should be made. Thanks!

#4 @hellofromTonya
6 months ago

  • Milestone changed from 5.8 to 5.9

Today is 5.8 Beta 1. Ran out of time for this one to land. Punting to 5.9 to give it time for code and test review and testing.

This ticket was mentioned in Slack in #core by chaion07. View the logs.


6 months ago

#6 @chaion07
6 months ago

  • Keywords dev-feedback added

Thanks @lalitjalandhar for reporting the issue. We reviewed this ticket during a [recent triage session at Core]https://wordpress.slack.com/archives/C02RQBWTW/p1623733904418100. As @hellofromTonya mentioned in here last comment that we need test review and testing as well as coding. We think that having an owner for this would also help determine the future of the ticket better. Also adding the dev-feedback keyword. Thanks!

#7 @TimothyBlynJacobs
4 weeks ago

  • Component changed from General to REST API
  • Version set to 4.4

#8 @TimothyBlynJacobs
4 weeks ago

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

In 51960:

REST API: Send a 500 status code when JSON encoding fails.

Previously, a 200 status code would be sent despite the 500 status code present in the response body.

Props hermpheus, lalitjalandhar.
Fixes #53056.

#9 @prbot
4 weeks ago

TimothyBJacobs commented on PR #1264:

Thanks for the patch @hermpheus! Merged in https://core.trac.wordpress.org/changeset/51960.

Note: See TracTickets for help on using tickets.