Allow a POST request to have an empty body #1786
Conversation
|
There's already an issue (maybe even a PR) on that IIRC |
|
👍 on my side, when the tests will be green. |
| || ('' === ($requestContent = $request->getContent()) && $request->isMethod('PUT')) | ||
| || ( | ||
| '' === ($requestContent = $request->getContent()) | ||
| && in_array($request->getMethod(), [Request::METHOD_POST, Request::METHOD_PUT]) |
There was a problem hiding this comment.
Can you use two === instead please (it's faster).
gorghoa
force-pushed
the
allow-empty-request-content-on-body-method
branch
from
9fdd5f9 to
8d40983
Compare
|
Thanks @dunglas, will add some tests. |
| || ( | ||
| '' === ($requestContent = $request->getContent()) | ||
| && (Request::METHOD_POST === $request->getMethod() || Request::METHOD_PUT === $request->getMethod()) | ||
| ) |
There was a problem hiding this comment.
Sure, Didn’t do it at first because I wasn’t sure if the mere idea of an empty POST would be accepted.
@soyuka, any idea what .feature file would be the coziest home for those tests?
this one: https://github.com/api-platform/core/blob/master/features/main/operation.feature ?
| || ('' === ($requestContent = $request->getContent()) && $request->isMethod('PUT')) | ||
| || ( | ||
| '' === ($requestContent = $request->getContent()) | ||
| && (Request::METHOD_POST === $request->getMethod() || Request::METHOD_PUT === $request->getMethod()) |
There was a problem hiding this comment.
Can you store the result of getMethod() in a variable to avoid an unnecessary function call? As it's in a listener it can be performance sensitive.
There was a problem hiding this comment.
done, thanks for the feedback
| if ( | ||
| $request->isMethodSafe(false) | ||
| || $request->isMethod('DELETE') | ||
| || Request::METHOD_DELETE === $method |
There was a problem hiding this comment.
Please, don't use Request::METHOD_* constants!
|
Unit tests updated. Let me know if a behat test is mandatory. |
|
Could you please rebase with master ? I think a behat test could be handy too! |
gorghoa
force-pushed
the
allow-empty-request-content-on-body-method
branch
from
2d89ecf to
a449fb2
Compare
gorghoa
force-pushed
the
allow-empty-request-content-on-body-method
branch
from
a449fb2 to
6d39e76
Compare
I'd like one 😄 |
|
thanks @Simperfit, rebased. Regarding the behat test: I think this PR promptly fixes an inconsistent behavior. It is unit tested. It’s a fast and sufficient test IMHO. I think the main issue – with more work/design to do – remains open, discussed here : #1282 |
|
Thanks @gorghoa. |
|
Looks like this wasn't applied to 2.2.x ? Anyway to have this merged in 2.2.x ? Because IMO this is more a bug than a new feature... |
|
It was considered a new feature, so not merged in |
|
Hmm... I've been bitten by this. Seems like this was a bad decision. |
|
See #2757 |
Hi,
A POST request with an empty body and a content-type set to json throws a
syntax errorexception at deserialization.This PR propose to allow empty content for POST request (just like empty content for
PUTis already ok).Didn’t find any strong indication to forbid empty post request by a quick search.
see also: http://lists.w3.org/Archives/Public/ietf-http-wg/2010JulSep/0272.html
(note, will write test if the idea is accepted ;) )
{ "@context": "/api/v2/contexts/Error", "@type": "hydra:Error", "hydra:title": "An error occurred", "hydra:description": "Syntax error", "trace": [ { "namespace": "", "short_class": "", "class": "", "type": "", "function": "", "file": "/var/www/w2c/api/vendor/symfony/serializer/Encoder/JsonDecode.php", "line": 78, "args": [] }, … ] }```