Support non-JSON ServerRequestInterface

[dsavina]

In #598, use of ServerRequestInterface::getParsedBody() has been replaced with inlined parsing of RequestInterface::getBody().

Later, it has been reintroduced (cf. #715), but for JSON requests only.

Therefore, this Pull Request aims to restore use of getParsedBody() when available, in fallback case as well (commonly application/x-www-form-urlencoded).

[spawnia]

Please add a test to ensure this actually fixes a reproducible issue and we do not cause a regression later.

[codecov]

Codecov Report

Merging #1004 (40d31cd) into master (717a5d5) will increase coverage by 0.24%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##             master    #1004      +/-   ##
============================================
+ Coverage     94.24%   94.49%   +0.24%     
  Complexity       50       50              
============================================
  Files           117      120       +3     
  Lines          9686     9635      -51     
============================================
- Hits           9129     9105      -24     
+ Misses          557      530      -27     

Impacted Files	Coverage Δ
src/Server/Helper.php	81.89% <85.71%> (-0.99%)	⬇️
src/Error/Warning.php	52.77% <0.00%> (-8.52%)	⬇️
src/Type/Definition/FieldArgument.php	51.06% <0.00%> (-7.43%)	⬇️
src/GraphQL.php	72.97% <0.00%> (-4.17%)	⬇️
src/Validator/Rules/KnownTypeNames.php	95.83% <0.00%> (-4.17%)	⬇️
src/Type/Definition/CustomScalarType.php	91.66% <0.00%> (-3.99%)	⬇️
src/Executor/Executor.php	72.97% <0.00%> (-3.50%)	⬇️
src/Type/Definition/EnumType.php	78.08% <0.00%> (-3.50%)	⬇️
src/Type/Definition/UnionType.php	93.18% <0.00%> (-2.17%)	⬇️
... and 76 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 717a5d5...40d31cd. Read the comment docs.

[dsavina]

Thanks for the feedback, I updated the code as requested.

I actually had an issue using Symfony's PsrHttpFactory, which, for some reason, ends up providing a ServerRequest with an empty body but a valid parsedBody.

Although this could be perceived as an issue on Symfony's side, considering that, in the case of an URL-encoded body:

• Method Helper::parseHttpRequest() relies on global variable $_POST (i.e. already parsed)
• Test method RequestParsingTest::parseRawFormUrlencodedRequest() indicates that reading from php://input should be prohibited

, I guess this behavior is nevertheless considered pretty standard, isn't it?

Lastly, in order to support these inconsistencies between body and parsedBody, I could propose this instead:

                if ($request instanceof ServerRequestInterface) {
                    $bodyParams = $request->getParsedBody();
                }

                $bodyParams ??= $this->decodeContent((string) $request->getBody(), $contentType[0]);

Would that be preferable?

[spawnia]

Would that be preferable?

If you can create a test case that fails with the current implementation and passes with that one, sure.

[spawnia]

Thanks for the PR and the thorough tests

[dsavina]

Hello @spawnia, do you have some insight regarding when this could be part of a new release, by any chance?

We will need this fix for our package thecodingmachine/graphqlite-bundle. If it's not something we can expect in the near future (which I would totally understand, I don't mean to push my personal agenda), I guess we could implement some temporary fix on our side. I'd still prefer not to if it's something we can avoid, though.

[spawnia]

Backported and releaed with https://github.com/webonyx/graphql-php/releases/tag/v14.11.2

[dsavina]

Nice, thank you very much!