Request::getBody(), see #57

[JanTvrdik]

[Majkl578]

Although it's a nice closure magic, I'm afraid it's gonna be slow, it's not even lazy... I mean, those unnecessary body calls could be easily avoided if type check was extracted outside (like addBodyCallback('application/json', $cb)).

[JanTvrdik]

@Majkl578 That's how I originally wanted to do it, but then I though that it is not powerful enough. What if you want to make decision based on something different than Content-Type (e.g. HTTP method)? What if you want to handle all something/* types? That being said I like the simple solution addBodyCallback('application/json', $cb) more than the current one.

[JanTvrdik]

Sidenote: it must somehow workaround the JSON decode DOS vulnerability.

[JanTvrdik]

I think that this is now ready to merge.

[enumag]

@JanTvrdik What about the JSON decode DOS vulnerability? I didn't see anything that would solve it in your code (but I might have missed it of course).

[JanTvrdik]

@enumag I ignore it the same way everybody in the PHP worlds ignores it. Those few that are concern may replace the body parser for JSON with sth smarter.

[JanTvrdik]

BTW: PSR7 calls this method getParsedBody() instead of just getBody(). It is longer but a bit more descriptive.

[dg]

It's a generous solution, but I wonder if it makes sense to have parsers something other than JSON and XML.

[JanTvrdik]

CSV, maybe.

[dg]

• getBody() vs getParsedBody(): imho second one is better
• support for application/x-www-form-urlencoded: nice, but IMHO useless
• support for application/json: really useful, but doesn't cover fact that Json::decode has option FORCE_ARRAY
• support for csv: imho not useful, but again - csv has not strong format, delimiter can be , or ;
• support for xml: very useful, has strong format, but - should we return SimpleXMLElement or DOMDocument?

The question is: should be this decised/configured in RequestFactory? Should it be ever done on HTTP level?

And next one: is getParsedBody() really useful when it can return anything? When it returns array, application must anyway check content-type header, whether it is JSON or CSV.

So I think that this is nice feature but not good abstraction. What about new generic function isContentType($type) or specific isJson() & isXml(), or even pragmatic getJsonBody($options)?