diff --git a/src/Server/Helper.php b/src/Server/Helper.php index f4ff7a8..621a060 100644 --- a/src/Server/Helper.php +++ b/src/Server/Helper.php @@ -37,7 +37,7 @@ class Helper if (!$doc instanceof DocumentNode) { $doc = Parser::parse($doc); } - if (!$op->allowsMutation() && AST::isMutation($op->operation, $doc)) { + if ($op->isReadOnly() && AST::isMutation($op->operation, $doc)) { throw new UserError("Cannot execute mutation in read-only context"); } @@ -133,47 +133,35 @@ class Helper return $validationRules; } - /** * Parses HTTP request and returns GraphQL QueryParams contained in this request. * For batched requests it returns an array of QueryParams. * - * @return OperationParams|OperationParams[] - */ - public function parseHttpRequest() - { - list ($parsedBody, $isReadonly) = $this->parseRawBody(); - return $this->toOperationParams($parsedBody, $isReadonly); - } - - /** - * Extracts parsed body and readonly flag from HTTP request + * This function doesn't check validity of these params. * * If $readRawBodyFn argument is not provided - will attempt to read raw request body from php://input stream * * @param callable|null $readRawBodyFn - * @return array + * @return OperationParams|OperationParams[] */ - public function parseRawBody(callable $readRawBodyFn = null) + public function parseHttpRequest(callable $readRawBodyFn = null) { $method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : null; if ($method === 'GET') { - $isReadonly = true; $request = array_change_key_case($_GET); if (isset($request['query']) || isset($request['queryid']) || isset($request['documentid'])) { - $body = $_GET; + $result = OperationParams::create($_GET, true); } else { throw new UserError('Cannot execute GET request without "query" or "queryId" parameter'); } } else if ($method === 'POST') { - $isReadonly = false; $contentType = isset($_SERVER['CONTENT_TYPE']) ? $_SERVER['CONTENT_TYPE'] : null; if (stripos($contentType, 'application/graphql') !== false) { $rawBody = $readRawBodyFn ? $readRawBodyFn() : $this->readRawBody(); - $body = ['query' => $rawBody ?: '']; + $result = OperationParams::create(['query' => $rawBody ?: '']); } else if (stripos($contentType, 'application/json') !== false) { $rawBody = $readRawBodyFn ? $readRawBodyFn() : $this->readRawBody(); $body = json_decode($rawBody ?: '', true); @@ -187,8 +175,17 @@ class Helper Utils::printSafeJson($body) ); } + if (isset($body[0])) { + $result = []; + foreach ($body as $index => $entry) { + $op = OperationParams::create($entry, true); + $result[] = $op; + } + } else { + $result = OperationParams::create($body); + } } else if (stripos($contentType, 'application/x-www-form-urlencoded') !== false) { - $body = $_POST; + $result = OperationParams::create($_POST); } else if (null === $contentType) { throw new UserError('Missing "Content-Type" header'); } else { @@ -197,41 +194,6 @@ class Helper } else { throw new UserError('HTTP Method "' . $method . '" is not supported', 405); } - return [ - $body, - $isReadonly - ]; - } - - /** - * Converts parsed body to OperationParams (or list of OperationParams for batched request) - * - * @param $parsedBody - * @param $isReadonly - * @return OperationParams|OperationParams[] - */ - public function toOperationParams($parsedBody, $isReadonly) - { - $assertValid = function (OperationParams $opParams, $queryNum = null) { - $errors = $opParams->validate(); - if (!empty($errors[0])) { - $err = $queryNum ? "Error in query #$queryNum: {$errors[0]}" : $errors[0]; - throw new UserError($err); - } - }; - - if (isset($parsedBody[0])) { - // Batched query - $result = []; - foreach ($parsedBody as $index => $entry) { - $op = OperationParams::create($entry, $isReadonly); - $assertValid($op, $index); - $result[] = $op; - } - } else { - $result = OperationParams::create($parsedBody, $isReadonly); - $assertValid($result); - } return $result; } @@ -243,46 +205,81 @@ class Helper return file_get_contents('php://input'); } + /** + * Checks validity of operation params and returns array of errors (empty array when params are valid) + * + * @param OperationParams $params + * @return array + */ + public function validateOperationParams(OperationParams $params) + { + $errors = []; + if (!$params->query && !$params->queryId) { + $errors[] = 'GraphQL Request must include at least one of those two parameters: "query" or "queryId"'; + } + if ($params->query && $params->queryId) { + $errors[] = 'GraphQL Request parameters "query" and "queryId" are mutually exclusive'; + } + + if ($params->query !== null && (!is_string($params->query) || empty($params->query))) { + $errors[] = 'GraphQL Request parameter "query" must be string, but got ' . + Utils::printSafeJson($params->query); + } + if ($params->queryId !== null && (!is_string($params->queryId) || empty($params->queryId))) { + $errors[] = 'GraphQL Request parameter "queryId" must be string, but got ' . + Utils::printSafeJson($params->queryId); + } + + if ($params->operation !== null && (!is_string($params->operation) || empty($params->operation))) { + $errors[] = 'GraphQL Request parameter "operation" must be string, but got ' . + Utils::printSafeJson($params->operation); + } + if ($params->variables !== null && (!is_array($params->variables) || isset($params->variables[0]))) { + $errors[] = 'GraphQL Request parameter "variables" must be object, but got ' . + Utils::printSafeJson($params->variables); + } + return $errors; + } + /** * Assertion to check that parsed body is valid instance of OperationParams (or array of instances) * - * @param $method - * @param $parsedBody + * @param OperationParams|OperationParams[] $parsedBody + * @throws InvariantViolation + * @throws UserError */ - public function assertBodyIsParsedProperly($method, $parsedBody) + public function assertValidRequest($parsedBody) { if (is_array($parsedBody)) { foreach ($parsedBody as $index => $entry) { if (!$entry instanceof OperationParams) { throw new InvariantViolation(sprintf( - '%s expects instance of %s or array of instances. Got invalid array where entry at position %d is %s', - $method, + 'GraphQL Server: Parsed http request must be an instance of %s or array of such instances, '. + 'but got invalid array where entry at position %d is %s', OperationParams::class, $index, Utils::printSafe($entry) )); } - $errors = $entry->validate(); + + $errors = $this->validateOperationParams($entry); if (!empty($errors[0])) { $err = $index ? "Error in query #$index: {$errors[0]}" : $errors[0]; - throw new InvariantViolation($err); + throw new UserError($err); } } - } - - if ($parsedBody instanceof OperationParams) { - $errors = $parsedBody->validate(); + } else if ($parsedBody instanceof OperationParams) { + $errors = $this->validateOperationParams($parsedBody); if (!empty($errors[0])) { - throw new InvariantViolation($errors[0]); + throw new UserError($errors[0]); } + } else { + throw new InvariantViolation(sprintf( + 'GraphQL Server: Parsed http request must be an instance of %s or array of such instances, but got %s', + OperationParams::class, + Utils::printSafe($parsedBody) + )); } - - throw new InvariantViolation(sprintf( - '%s expects instance of %s or array of instances, but got %s', - $method, - OperationParams::class, - Utils::printSafe($parsedBody) - )); } } diff --git a/src/Server/OperationParams.php b/src/Server/OperationParams.php index e81982d..d701f4f 100644 --- a/src/Server/OperationParams.php +++ b/src/Server/OperationParams.php @@ -39,17 +39,17 @@ class OperationParams /** * @var bool */ - private $allowsMutations; + private $readOnly; /** * Creates an instance from given array * * @param array $params - * @param bool $allowsMutations + * @param bool $readonly * * @return static */ - public static function create(array $params, $allowsMutations = true) + public static function create(array $params, $readonly = false) { $instance = new static(); $instance->originalInput = $params; @@ -68,7 +68,7 @@ class OperationParams $instance->queryId = $params['queryid'] ?: $params['documentid']; $instance->operation = $params['operation']; $instance->variables = $params['variables']; - $instance->allowsMutations = (bool) $allowsMutations; + $instance->readOnly = (bool) $readonly; return $instance; } @@ -76,36 +76,6 @@ class OperationParams /** * @return array */ - public function validate() - { - $errors = []; - if (!$this->query && !$this->queryId) { - $errors[] = 'GraphQL Request must include at least one of those two parameters: "query" or "queryId"'; - } - if ($this->query && $this->queryId) { - $errors[] = 'GraphQL Request parameters "query" and "queryId" are mutually exclusive'; - } - - if ($this->query !== null && (!is_string($this->query) || empty($this->query))) { - $errors[] = 'GraphQL Request parameter "query" must be string, but got ' . - Utils::printSafeJson($this->query); - } - if ($this->queryId !== null && (!is_string($this->queryId) || empty($this->queryId))) { - $errors[] = 'GraphQL Request parameter "queryId" must be string, but got ' . - Utils::printSafeJson($this->queryId); - } - - if ($this->operation !== null && (!is_string($this->operation) || empty($this->operation))) { - $errors[] = 'GraphQL Request parameter "operation" must be string, but got ' . - Utils::printSafeJson($this->operation); - } - if ($this->variables !== null && (!is_array($this->variables) || isset($this->variables[0]))) { - $errors[] = 'GraphQL Request parameter "variables" must be object, but got ' . - Utils::printSafeJson($this->variables); - } - return $errors; - } - public function getOriginalInput() { return $this->originalInput; @@ -114,8 +84,8 @@ class OperationParams /** * @return bool */ - public function allowsMutation() + public function isReadOnly() { - return $this->allowsMutations; + return $this->readOnly; } } diff --git a/src/Server/StandardServer.php b/src/Server/StandardServer.php index 632dd0f..2cd0c1c 100644 --- a/src/Server/StandardServer.php +++ b/src/Server/StandardServer.php @@ -54,10 +54,9 @@ class StandardServer public function executeRequest($parsedBody = null) { if (null !== $parsedBody) { - $this->helper->assertBodyIsParsedProperly(__METHOD__, $parsedBody); - } else { $parsedBody = $this->helper->parseHttpRequest(); } + $this->helper->assertValidRequest($parsedBody); $batched = is_array($parsedBody); diff --git a/tests/Server/RequestParsingTest.php b/tests/Server/RequestParsingTest.php index 62ad5a1..d47e5e9 100644 --- a/tests/Server/RequestParsingTest.php +++ b/tests/Server/RequestParsingTest.php @@ -1,6 +1,7 @@ parseRawRequest('application/graphql', $query); + $parsedBody = $this->parseRawRequest('application/graphql', $query); - $this->assertSame(['query' => $query], $parsedBody); - $this->assertFalse($isReadonly); + $this->assertValidOperationParams($parsedBody, $query); + $this->assertFalse($parsedBody->isReadOnly()); } - public function testParsesSimpleUrlencodedRequest() + public function testParsesUrlencodedRequest() { $query = '{my query}'; $variables = ['test' => 1, 'test2' => 2]; @@ -31,12 +32,12 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase 'operation' => $operation ]; - list ($parsedBody, $isReadonly) = $this->parseFormUrlencodedRequest($post); - $this->assertSame($post, $parsedBody); - $this->assertFalse($isReadonly); + $parsedBody = $this->parseFormUrlencodedRequest($post); + $this->assertValidOperationParams($parsedBody, $query, null, $variables, $operation); + $this->assertFalse($parsedBody->isReadOnly()); } - public function testParsesSimpleGETRequest() + public function testParsesGetRequest() { $query = '{my query}'; $variables = ['test' => 1, 'test2' => 2]; @@ -48,12 +49,12 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase 'operation' => $operation ]; - list ($parsedBody, $isReadonly) = $this->parseGetRequest($get); - $this->assertSame($get, $parsedBody); - $this->assertTrue($isReadonly); + $parsedBody = $this->parseGetRequest($get); + $this->assertValidOperationParams($parsedBody, $query, null, $variables, $operation); + $this->assertTrue($parsedBody->isReadonly()); } - public function testParsesSimpleJSONRequest() + public function testParsesJSONRequest() { $query = '{my query}'; $variables = ['test' => 1, 'test2' => 2]; @@ -64,10 +65,9 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase 'variables' => $variables, 'operation' => $operation ]; - - list ($parsedBody, $isReadonly) = $this->parseRawRequest('application/json', json_encode($body)); - $this->assertEquals($body, $parsedBody); - $this->assertFalse($isReadonly); + $parsedBody = $this->parseRawRequest('application/json', json_encode($body)); + $this->assertValidOperationParams($parsedBody, $query, null, $variables, $operation); + $this->assertFalse($parsedBody->isReadOnly()); } public function testParsesBatchJSONRequest() @@ -85,9 +85,16 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase ], ]; - list ($parsedBody, $isReadonly) = $this->parseRawRequest('application/json', json_encode($body)); - $this->assertEquals($body, $parsedBody); - $this->assertFalse($isReadonly); + $parsedBody = $this->parseRawRequest('application/json', json_encode($body)); + $this->assertInternalType('array', $parsedBody); + $this->assertCount(2, $parsedBody); + + $this->assertValidOperationParams($parsedBody[0], $body[0]['query'], null, $body[0]['variables'], $body[0]['operation']); + $this->assertValidOperationParams($parsedBody[1], null, $body[1]['queryId'], $body[1]['variables'], $body[1]['operation']); + + // Batched queries must be read-only (do not allow batched mutations) + $this->assertTrue($parsedBody[0]->isReadOnly()); + $this->assertTrue($parsedBody[1]->isReadOnly()); } public function testFailsParsingInvalidJsonRequest() @@ -130,162 +137,6 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase $this->parseRawRequest(null, 'test', "PUT"); } - public function testSimpleRequestShouldPass() - { - $query = '{my q}'; - $variables = ['a' => 'b', 'c' => 'd']; - $operation = 'op'; - - $parsedBody = [ - 'query' => $query, - 'variables' => $variables, - 'operation' => $operation, - ]; - - $helper = new Helper(); - $params = $helper->toOperationParams($parsedBody, false); - $this->assertValidOperationParams($params, $query, null, $variables, $operation); - } - - public function testRequestWithQueryIdShouldPass() - { - $queryId = 'some-query-id'; - $variables = ['a' => 'b', 'c' => 'd']; - $operation = 'op'; - - $parsedBody = [ - 'queryId' => $queryId, - 'variables' => $variables, - 'operation' => $operation, - ]; - - $helper = new Helper(); - $params = $helper->toOperationParams($parsedBody, false); - $this->assertValidOperationParams($params, null, $queryId, $variables, $operation); - } - - public function testProducesCorrectOperationParamsForBatchRequest() - { - $query = '{my q}'; - $queryId = 'some-query-id'; - $variables = ['a' => 'b', 'c' => 'd']; - $operation = 'op'; - - $parsedBody = [ - [ - 'query' => $query, - 'variables' => $variables, - 'operation' => $operation, - ], - [ - 'queryId' => $queryId, - 'variables' => [], - 'operation' => null - ], - ]; - - $helper = new Helper(); - $params = $helper->toOperationParams($parsedBody, false); - - $this->assertTrue(is_array($params)); - $this->assertValidOperationParams($params[0], $query, null, $variables, $operation); - $this->assertValidOperationParams($params[1], null, $queryId, [], null); - } - - public function testRequiresQueryOrQueryId() - { - $parsedBody = [ - 'variables' => ['foo' => 'bar'], - 'operation' => 'op', - ]; - - $helper = new Helper(); - - $this->setExpectedException( - UserError::class, - 'GraphQL Request must include at least one of those two parameters: "query" or "queryId"' - ); - $helper->toOperationParams($parsedBody, false); - } - - public function testFailsWhenBothQueryAndQueryIdArePresent() - { - $parsedBody = [ - 'query' => '{my query}', - 'queryId' => 'my-query-id', - ]; - - $helper = new Helper(); - - $this->setExpectedException( - UserError::class, - 'GraphQL Request parameters "query" and "queryId" are mutually exclusive' - ); - $helper->toOperationParams($parsedBody, false); - } - - public function testFailsWhenQueryParameterIsNotString() - { - $parsedBody = [ - 'query' => ['t' => '{my query}'] - ]; - - $helper = new Helper(); - - $this->setExpectedException( - UserError::class, - 'GraphQL Request parameter "query" must be string, but got object with first key: "t"' - ); - $helper->toOperationParams($parsedBody, false); - } - - public function testFailsWhenQueryIdParameterIsNotString() - { - $parsedBody = [ - 'queryId' => ['t' => '{my query}'] - ]; - - $helper = new Helper(); - - $this->setExpectedException( - UserError::class, - 'GraphQL Request parameter "queryId" must be string, but got object with first key: "t"' - ); - $helper->toOperationParams($parsedBody, false); - } - - public function testFailsWhenOperationParameterIsNotString() - { - $parsedBody = [ - 'query' => '{my query}', - 'operation' => [] - ]; - - $helper = new Helper(); - - $this->setExpectedException( - UserError::class, - 'GraphQL Request parameter "operation" must be string, but got array(0)' - ); - $helper->toOperationParams($parsedBody, false); - } - - public function testFailsWhenVariablesParameterIsNotObject() - { - $parsedBody = [ - 'query' => '{my query}', - 'variables' => 'test' - ]; - - $helper = new Helper(); - - $this->setExpectedException( - UserError::class, - 'GraphQL Request parameter "variables" must be object, but got "test"' - ); - $helper->toOperationParams($parsedBody, false); - } - /** * @param string $contentType * @param string $content @@ -299,7 +150,7 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase $_SERVER['REQUEST_METHOD'] = $method; $helper = new Helper(); - return $helper->parseRawBody(function() use ($content) { + return $helper->parseHttpRequest(function() use ($content) { return $content; }); } @@ -315,12 +166,14 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase $_POST = $postValue; $helper = new Helper(); - return $helper->parseRawBody(); + return $helper->parseHttpRequest(function() { + throw new InvariantViolation("Shouldn't read from php://input for urlencoded request"); + }); } /** * @param $getValue - * @return array + * @return OperationParams */ private function parseGetRequest($getValue) { @@ -328,7 +181,9 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase $_GET = $getValue; $helper = new Helper(); - return $helper->parseRawBody(); + return $helper->parseHttpRequest(function() { + throw new InvariantViolation("Shouldn't read from php://input for urlencoded request"); + }); } /** diff --git a/tests/Server/RequestValidationTest.php b/tests/Server/RequestValidationTest.php new file mode 100644 index 0000000..4f0c2ae --- /dev/null +++ b/tests/Server/RequestValidationTest.php @@ -0,0 +1,179 @@ + 'b', 'c' => 'd']; + $operation = 'op'; + + $parsedBody = OperationParams::create([ + 'query' => $query, + 'variables' => $variables, + 'operation' => $operation, + ]); + + $this->assertValid($parsedBody); + } + + public function testRequestWithQueryIdShouldValidate() + { + $queryId = 'some-query-id'; + $variables = ['a' => 'b', 'c' => 'd']; + $operation = 'op'; + + $parsedBody = OperationParams::create([ + 'queryId' => $queryId, + 'variables' => $variables, + 'operation' => $operation, + ]); + + $this->assertValid($parsedBody); + } + + public function testBatchRequestShouldValidate() + { + $query = '{my q}'; + $queryId = 'some-query-id'; + $variables = ['a' => 'b', 'c' => 'd']; + $operation = 'op'; + + $parsedBody = [ + OperationParams::create([ + 'query' => $query, + 'variables' => $variables, + 'operation' => $operation, + ]), + OperationParams::create([ + 'queryId' => $queryId, + 'variables' => [], + 'operation' => null + ]), + ]; + + $this->assertValid($parsedBody); + } + + public function testThrowsOnInvalidRequest() + { + $parsedBody = 'str'; + $this->assertInternalError( + $parsedBody, + 'GraphQL Server: Parsed http request must be an instance of GraphQL\Server\OperationParams or array of such instances, but got "str"' + ); + + $parsedBody = ['str']; + $this->assertInternalError( + $parsedBody, + 'GraphQL Server: Parsed http request must be an instance of GraphQL\Server\OperationParams or array of such instances, but got invalid array where entry at position 0 is "str"' + ); + } + + public function testRequiresQueryOrQueryId() + { + $parsedBody = OperationParams::create([ + 'variables' => ['foo' => 'bar'], + 'operation' => 'op', + ]); + + $this->assertInputError( + $parsedBody, + 'GraphQL Request must include at least one of those two parameters: "query" or "queryId"' + ); + } + + public function testFailsWhenBothQueryAndQueryIdArePresent() + { + $parsedBody = OperationParams::create([ + 'query' => '{my query}', + 'queryId' => 'my-query-id', + ]); + + $this->assertInputError( + $parsedBody, + 'GraphQL Request parameters "query" and "queryId" are mutually exclusive' + ); + } + + public function testFailsWhenQueryParameterIsNotString() + { + $parsedBody = OperationParams::create([ + 'query' => ['t' => '{my query}'] + ]); + + $this->assertInputError( + $parsedBody, + 'GraphQL Request parameter "query" must be string, but got object with first key: "t"' + ); + } + + public function testFailsWhenQueryIdParameterIsNotString() + { + $parsedBody = OperationParams::create([ + 'queryId' => ['t' => '{my query}'] + ]); + + $this->assertInputError( + $parsedBody, + 'GraphQL Request parameter "queryId" must be string, but got object with first key: "t"' + ); + } + + public function testFailsWhenOperationParameterIsNotString() + { + $parsedBody = OperationParams::create([ + 'query' => '{my query}', + 'operation' => [] + ]); + + $this->assertInputError( + $parsedBody, + 'GraphQL Request parameter "operation" must be string, but got array(0)' + ); + } + + public function testFailsWhenVariablesParameterIsNotObject() + { + $parsedBody = OperationParams::create([ + 'query' => '{my query}', + 'variables' => 'test' + ]); + + $this->assertInputError($parsedBody, 'GraphQL Request parameter "variables" must be object, but got "test"'); + } + + private function assertValid($parsedRequest) + { + $helper = new Helper(); + $helper->assertValidRequest($parsedRequest); + } + + private function assertInputError($parsedRequest, $expectedMessage) + { + try { + $helper = new Helper(); + $helper->assertValidRequest($parsedRequest); + $this->fail('Expected exception not thrown'); + } catch (UserError $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + } + } + + private function assertInternalError($parsedRequest, $expectedMessage) + { + try { + $helper = new Helper(); + $helper->assertValidRequest($parsedRequest); + $this->fail('Expected exception not thrown'); + } catch (InvariantViolation $e) { + $this->assertEquals($expectedMessage, $e->getMessage()); + } + } +}