From 178b179db3b4fd6d0d9d5499ee99f9005b7ba415 Mon Sep 17 00:00:00 2001 From: Adrien Crivelli Date: Thu, 21 Dec 2017 15:01:57 +0900 Subject: [PATCH] Drop support non pre-parsed PSR-7 request body This revert #202 (commit 9d37f4c) because trying to parse PSR-7 request was a mistake. The whole point of PSR-7 is to allow for interoperability and be able to use specialized libs for body parsing (amongst many other things). Trying to parse ourselves would be opening a can of worm if/when other content types have to be supported. It is more correct and future safe to require that the body is parsed before being passed to GraphQL. --- src/Server/Helper.php | 15 ++++++--------- tests/Server/RequestParsingTest.php | 22 ++++++++++++++++++++-- tests/Server/StandardServerTest.php | 24 +++++------------------- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/Server/Helper.php b/src/Server/Helper.php index 8cc13f0..8bc8588 100644 --- a/src/Server/Helper.php +++ b/src/Server/Helper.php @@ -493,21 +493,18 @@ class Helper ); } - // Try parsing ourselves if PSR-7 implementation doesn't parse JSON automatically - if (is_array($bodyParams) && empty($bodyParams)) { - $bodyParams = json_decode($request->getBody(), true); - - if (json_last_error()) { - throw new RequestError("Could not parse JSON: " . json_last_error_msg()); - } - } - if (!is_array($bodyParams)) { throw new RequestError( "GraphQL Server expects JSON object or array, but got " . Utils::printSafeJson($bodyParams) ); } + + if (empty($bodyParams)) { + throw new InvariantViolation( + "PSR-7 request is expected to provide parsed body for \"application/json\" requests but got empty array" + ); + } } else { $bodyParams = $request->getParsedBody(); diff --git a/tests/Server/RequestParsingTest.php b/tests/Server/RequestParsingTest.php index c26922c..303f3ae 100644 --- a/tests/Server/RequestParsingTest.php +++ b/tests/Server/RequestParsingTest.php @@ -181,6 +181,20 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase } } + public function testFailsParsingNonPreParsedPsrRequest() + { + try { + $this->parsePsrRequest('application/json', json_encode([])); + $this->fail('Expected exception not thrown'); + } catch (InvariantViolation $e) { + // Expecting parsing exception to be thrown somewhere else: + $this->assertEquals( + 'PSR-7 request is expected to provide parsed body for "application/json" requests but got empty array', + $e->getMessage() + ); + } + } + // There is no equivalent for psr request, because it should throw public function testFailsParsingNonArrayOrObjectJsonRequest() @@ -242,15 +256,19 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase public function testFailsOnMethodsOtherThanPostOrGet() { + $body = [ + 'query' => '{my query}', + ]; + try { - $this->parseRawRequest('application/json', json_encode([]), "PUT"); + $this->parseRawRequest('application/json', json_encode($body), "PUT"); $this->fail('Expected exception not thrown'); } catch (RequestError $e) { $this->assertEquals('HTTP Method "PUT" is not supported', $e->getMessage()); } try { - $this->parsePsrRequest('application/json', json_encode([]), "PUT"); + $this->parsePsrRequest('application/json', json_encode($body), "PUT"); $this->fail('Expected exception not thrown'); } catch (RequestError $e) { $this->assertEquals('HTTP Method "PUT" is not supported', $e->getMessage()); diff --git a/tests/Server/StandardServerTest.php b/tests/Server/StandardServerTest.php index 2b1055e..5a1246d 100644 --- a/tests/Server/StandardServerTest.php +++ b/tests/Server/StandardServerTest.php @@ -43,9 +43,9 @@ class StandardServerTest extends TestCase public function testSimplePsrRequestExecution() { - $body = json_encode([ + $body = [ 'query' => '{f1}' - ]); + ]; $expected = [ 'data' => [ @@ -53,11 +53,8 @@ class StandardServerTest extends TestCase ] ]; - $preParsedRequest = $this->preparePsrRequest('application/json', $body, true); - $this->assertPsrRequestEquals($expected, $preParsedRequest); - - $notPreParsedRequest = $this->preparePsrRequest('application/json', $body, false); - $this->assertPsrRequestEquals($expected, $notPreParsedRequest); + $request = $this->preparePsrRequest('application/json', $body); + $this->assertPsrRequestEquals($expected, $request); } private function executePsrRequest($psrRequest) @@ -75,22 +72,11 @@ class StandardServerTest extends TestCase return $result; } - private function preparePsrRequest($contentType, $content, $preParseBody) + private function preparePsrRequest($contentType, $parsedBody) { - $psrRequestBody = new PsrStreamStub(); - $psrRequestBody->content = $content; - $psrRequest = new PsrRequestStub(); $psrRequest->headers['content-type'] = [$contentType]; $psrRequest->method = 'POST'; - $psrRequest->body = $psrRequestBody; - - if ($preParseBody && $contentType === 'application/json') { - $parsedBody = json_decode($content, true); - } else { - $parsedBody = []; - } - $psrRequest->parsedBody = $parsedBody; return $psrRequest; }