Empty body on custom operations

[soyuka]

1. Keep 400 status code on standard operations when an empty body is sent (not valid json)
2. Allow empty body on custom operations (POST/PUT/DELETE)

Ref: #1274 #782 and #805. #1786

[dunglas]

It looks weird to me to not have a different behavior for custom and built in operations.

What about this one: allow empty requests for a all operations IF Content-Type is not set?

[soyuka]

Only thing I know for sure is that sending an empty body on our operations should return a 400 status code because it's invalid (i.e. empty string is not valid json ; Content-Type is indeed a json-like one).

Now, there are users who may want to use empty contents (on POST/PUT operations). Forcing them to not use a Content-Type to be able to do so looks bad for DX no?

[gorghoa]

At least, it should be consistent between PUT and POST (not the case right know I reckon)

[gorghoa]

Now, there are users who may want to use empty contents (on POST/PUT operations). Forcing them to not use a Content-Type to be able to do so looks bad for DX no?

I have to admit that from my point of view, it rings a bell 👍

[gorghoa]

What about this one: allow empty requests for a all operations IF Content-Type is not set?

@dunglas, does it even work regarding content negotiation?

Sorry, was thinking about this while reading bedtime stories, seems now totally irrelevant ;)

[dunglas]

I'm 👎 to allow empty bodies if the content-type is set to JSON or JSON-LD. An empty string is not a valid JSON document (you can try with https://jsonlint.com/) but the client is advertising that the payload is valid JSON. To me it should generate a 400.

Validating the content-type is an OWASP best practice.

[sroze]

I'm 👎 to allow empty bodies if the content-type is set to JSON or JSON-LD.

Agree. Though I would 👍 very very much without a Content-Type.

[gorghoa]

Wokay! I have to verify to be sure that PUT currently allow empty body while content-type is set, but if so, shouldn't it be the same for PUT then? (BC breaking…)

[dunglas]

I don't think we allow it for PUT neither.

[dunglas]

It seems to be OK if there is no body and no content-type set: https://stackoverflow.com/a/29784642/1352334

[gorghoa]

I don't think we allow it for PUT neither.

@dunglas , I was assuming PUT was allowed to skip deserialization of empty content based on https://github.com/api-platform/core/blob/master/src/EventListener/DeserializeListener.php#L55

I may very well be missing something though, I am not very comfortable with the codebase yet :)

[gorghoa]

Some facts I experiment with my setup (api-p v2.2.2):

note: all requests have been made with curl -X POST|PUT http://my-endpoint.tcho/api [-H 'Content-Type: application/json'] [--data '{}'])

method	content-type	body content	resulting http code	notes
PUT	not set	empty	200
PUT	application/json	empty	200
PUT	application/json	{}	200
POST	not set	empty	406	not acceptable: content-type mandatory
POST	application/json	empty	400	Bad request, syntax error
POST	application/json	{}	200	Ok