brefphp / bref

Serverless PHP on AWS Lambda
https://bref.sh
MIT License
3.14k stars 364 forks source link

Event body parsing issue #1

Closed radutopala closed 6 years ago

radutopala commented 6 years ago

@mnapoli I could only use the httpHandler after applying this patch, else $request was containing a gibberish array.. maybe it's only me..

Try with any of the json from https://apex.sh/docs/ping/webhooks/.

diff --git a/src/Bridge/Psr7/RequestFactory.php b/src/Bridge/Psr7/RequestFactory.php
index 8efae90..cd3e490 100644
--- a/src/Bridge/Psr7/RequestFactory.php
+++ b/src/Bridge/Psr7/RequestFactory.php
@@ -17,7 +17,7 @@ class RequestFactory
     {
         $method = $event['httpMethod'] ?? 'GET';
         $query = $event['queryStringParameters'] ?? [];
-        parse_str($event['body'] ?? '', $request);
+        $request = $event['body'] ?? '';
         $files = [];
         $uri = $event['requestContext']['path'] ?? '/';
         $headers = $event['headers'] ?? [];

Else, thanks for this wrapper 😄

mnapoli commented 6 years ago

Thanks! That's weird, I'm using POST requests on another project and that worked fine. I will do additional tests.

Oh wait maybe I should do parse_str only for specific POST requests. E.g. if it's a POST request but JSON is sent then you don't want the body to be parsed. I think that may be it.

radutopala commented 6 years ago

Yeah, that should be it. I sent the json directly in the body.

mnapoli commented 6 years ago

The approach of Zend Expressive is interesting, they have a middleware that will apply the correct "strategy" of decoding the body based on the header: https://github.com/zendframework/zend-expressive-helpers/blob/master/src/BodyParams/BodyParamsMiddleware.php The downside is that it's a middleware, and JSON decoding is not really something that PHP does natively. I don't want this package to do more than what PHP does natively, frameworks already deal with JSON.

Slim's approach in it's base PSR-7 implementation is simpler:

https://github.com/slimphp/Slim-Http/blob/305e7572084d6476939ffd9f71e388ab847ecbc3/src/Request.php#L122-L127

radutopala commented 6 years ago

Slim's implementation looks natural and straightforward indeed.

mnapoli commented 6 years ago

2 fixes this problem.