yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

Need to return empty body in JSON response on 204 status (no content) #17094

Closed xfuturomax closed 5 years ago

xfuturomax commented 5 years ago

Now, if response format is JSON (RESTfull API) and there is no content, i.e null, yii responds string 'null', and this is not right.Some software like Postman doesn't understand this.

/yiisoft/yii2/web/JsonResponseFormatter.php:121 elseif ($response->content === null) { $response->content = 'null'; }

samdark commented 5 years ago

Is that true for all request types?

xfuturomax commented 5 years ago

For DELETE requests certainly

np25071984 commented 5 years ago

Can we just replace 'null' string to empty one for all types? What is the reason we didn't do it at very beginning?

https://github.com/yiisoft/yii2/blob/39d3b6519cf37353cf372f8370792ed4171fc780/tests/framework/web/JsonResponseFormatterTest.php#L191-L196

samdark commented 5 years ago

14759

xfuturomax commented 5 years ago

The 204 response must respond with an empty body, this is a priority over the json format specification.

np25071984 commented 5 years ago

Only 204 or we can extend this list?

    protected function formatJson($response)
    {
        if ($response->data !== null) {
            $options = $this->encodeOptions;
            if ($this->prettyPrint) {
                $options |= JSON_PRETTY_PRINT;
            }
            $response->content = Json::encode($response->data, $options);
        } elseif ($response->content === null) {
            if (in_array($response->getStatusCode(), [204])) {
                $response->content = '';
            } else {
                $response->content = 'null';
            }
        }
    }
samdark commented 5 years ago

Not sending content on 204 response doesn't depend on formatter and should be done regardless in Response::sendContent() like:

if ($this->getStatusCode() === 204) {
    return;
}
samdark commented 5 years ago

The case should be covered with a test.

lourdas commented 5 years ago

The null response broke yiisoft/yii2-jui's AutoComplete object. A extra null was appended in the JSON response. Downgrading to 2.0.15.1 confirmed the issue is with 2.0.16. Will apply the patch and try again.

lourdas commented 5 years ago

Yes, the problem remains for me. The AutoComplete widget expects a json object to display the popup menu. The object is returned, but there's also a null string at the end of it (if I pass the response through console.log).

And the status code in that case is 200.

np25071984 commented 5 years ago

Why didn't you start new issue?

As I can understand, the problem is about response content (not about AutoComplete widget, it is important). There is "null string at the end" appears somehow. So, let us see your generating code with input params.

lourdas commented 5 years ago

I didn't start a new issue, because I thought it's related to this issue.