Gpenverne fix/allow empty content on post and put requests

[dunglas]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#781, #782
License	MIT
Doc PR	n/a

Follows #782. All credit go to @gpenverne.

[soyuka]

👍

[teohhanhui]

This scenario is incorrect. This is not a custom operation, and I don't suppose it should return 200 when the semantics is undefined (for the default PUT operation). Shouldn't it be 400 instead?

[dunglas]

Indeed it is incorrect. But why a 400 if we allow empty requests for PUT, it should be allowed for all PUT operations isn't it?

[teohhanhui]

What does PUT with an empty body mean? The semantics are undefined. The
purpose of this PR is to skip deserialization of empty body for custom
operations, but it should return 400 for the default POST / PUT operations.

On Tue, 18 Oct 2016, 13:52 Kévin Dunglas, notifications@github.com wrote:

@dunglas commented on this pull request.

In features/crud.feature #805:

@@ -358,6 +358,9 @@ Feature: Create-Retrieve-Update-Delete
"alias": null
}
"""

• Scenario: PUT request on custom operations without content

Indeed it is incorrect. But why a 400 if we allow empty requests for PUT,
it should be allowed for all PUT operations isn't it?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#805, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAhf67ArDO9s_HVLNJySGDfi-NWC_sqHks5q1F6ngaJpZM4KYlj_
.

[dunglas]

Ok got what you mean. Not sure that we should add this in core. Maybe just a doc to explain how to do that would be enough? It's weird to have a different behavior for built-in and user-land operations.

[teohhanhui]

It's weird to have a different behavior for built-in and user-land operations.

Ideally it should return 400 for both built-in and custom POST / PUT operations when an empty body is encountered, unless the user handles it in the custom operation. But yeah, I'm not sure how that can be implemented. So probably we should merge this as is.

[kbsali]

@teohhanhui why should it return a 400? Sending empty content is not an error!

[teohhanhui]

@kbsali:

why should it return a 400? Sending empty content is not an error!

It really depends on the content type. For example, an empty string is NOT valid JSON.

[jdeniau]

Hi @soyuka or @dunglas ,

What is the status of this PR ? empty content has been allowed on PUT request via #1274 .

@teohhanhui : You are right that empty string is an invalid JSON but POST method do send empty string as body if you don't specify one.

For me this should be iso between all "unsafe" methods (except DELETE), if you send an error you should send errors on PUT, POST, PATCH, etc., but if you don't send an error, you should not send error at all.

[soyuka]

@jdeniau I think it'll be resolved once every point I stated in #1282 are. Maybe we should close this in favor of #1282.