From f8c3195e54b5087601348392ef4e0c5032f6c1e5 Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Sun, 16 Jul 2017 18:52:38 +0700 Subject: [PATCH] Granular methods for HTTP request parsing + tests --- src/Server/Helper.php | 179 ++++++++++---- src/Server/OperationParams.php | 23 +- src/Server/StandardServer.php | 53 +---- src/Utils/Utils.php | 47 +++- tests/Server/QueryExecutionTest.php | 7 +- tests/Server/RequestParsingTest.php | 350 ++++++++++++++++++++++++++++ 6 files changed, 558 insertions(+), 101 deletions(-) create mode 100644 tests/Server/RequestParsingTest.php diff --git a/src/Server/Helper.php b/src/Server/Helper.php index f863908..f4ff7a8 100644 --- a/src/Server/Helper.php +++ b/src/Server/Helper.php @@ -28,7 +28,7 @@ class Helper * * @return ExecutionResult|Promise */ - public static function executeOperation(ServerConfig $config, OperationParams $op) + public function executeOperation(ServerConfig $config, OperationParams $op) { $phpErrors = []; $execute = function() use ($config, $op) { @@ -83,8 +83,12 @@ class Helper * @param OperationParams $op * @return string|DocumentNode */ - private static function loadPersistedQuery(ServerConfig $config, OperationParams $op) + public function loadPersistedQuery(ServerConfig $config, OperationParams $op) { + if (!$op->queryId) { + throw new InvariantViolation("Could not load persisted query: queryId is not set"); + } + // Load query if we got persisted query id: $loader = $config->getPersistentQueryLoader(); @@ -110,7 +114,7 @@ class Helper * @param OperationParams $params * @return array */ - private static function resolveValidationRules(ServerConfig $config, OperationParams $params) + public function resolveValidationRules(ServerConfig $config, OperationParams $params) { // Allow customizing validation rules per operation: $validationRules = $config->getValidationRules(); @@ -129,16 +133,85 @@ 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 static function parseHttpRequest() + public function parseHttpRequest() { - $contentType = isset($_SERVER['CONTENT_TYPE']) ? $_SERVER['CONTENT_TYPE'] : null; + list ($parsedBody, $isReadonly) = $this->parseRawBody(); + return $this->toOperationParams($parsedBody, $isReadonly); + } + /** + * Extracts parsed body and readonly flag from HTTP request + * + * If $readRawBodyFn argument is not provided - will attempt to read raw request body from php://input stream + * + * @param callable|null $readRawBodyFn + * @return array + */ + public function parseRawBody(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; + } 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 ?: '']; + } else if (stripos($contentType, 'application/json') !== false) { + $rawBody = $readRawBodyFn ? $readRawBodyFn() : $this->readRawBody(); + $body = json_decode($rawBody ?: '', true); + + if (json_last_error()) { + throw new UserError("Could not parse JSON: " . json_last_error_msg()); + } + if (!is_array($body)) { + throw new UserError( + "GraphQL Server expects JSON object or array, but got " . + Utils::printSafeJson($body) + ); + } + } else if (stripos($contentType, 'application/x-www-form-urlencoded') !== false) { + $body = $_POST; + } else if (null === $contentType) { + throw new UserError('Missing "Content-Type" header'); + } else { + throw new UserError("Unexpected content type: " . Utils::printSafeJson($contentType)); + } + } 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])) { @@ -147,45 +220,69 @@ class Helper } }; - if (stripos($contentType, 'application/graphql' !== false)) { - $body = file_get_contents('php://input') ?: ''; - $op = OperationParams::create(['query' => $body]); - $assertValid($op); - } else if (stripos($contentType, 'application/json') !== false || stripos($contentType, 'text/json') !== false) { - $body = file_get_contents('php://input') ?: ''; - $data = json_decode($body, true); - - if (json_last_error()) { - throw new UserError("Could not parse JSON: " . json_last_error_msg()); + if (isset($parsedBody[0])) { + // Batched query + $result = []; + foreach ($parsedBody as $index => $entry) { + $op = OperationParams::create($entry, $isReadonly); + $assertValid($op, $index); + $result[] = $op; } - if (!is_array($data)) { - throw new UserError( - "GraphQL Server expects JSON object or array, but got %s" . - Utils::printSafe($data) - ); - } - if (isset($data[0])) { - $op = []; - foreach ($data as $index => $entry) { - $params = OperationParams::create($entry); - $assertValid($params, $index); - $op[] = $params; - } - } else { - $op = OperationParams::create($data); - $assertValid($op); - } - } else if (stripos($contentType, 'application/x-www-form-urlencoded') !== false) { - if ($_SERVER['REQUEST_METHOD'] === 'GET') { - $op = OperationParams::create($_GET, false); - } else { - $op = OperationParams::create($_POST); - } - $assertValid($op); } else { - throw new UserError("Bad request: unexpected content type: " . Utils::printSafe($contentType)); + $result = OperationParams::create($parsedBody, $isReadonly); + $assertValid($result); + } + return $result; + } + + /** + * @return bool|string + */ + public function readRawBody() + { + return file_get_contents('php://input'); + } + + /** + * Assertion to check that parsed body is valid instance of OperationParams (or array of instances) + * + * @param $method + * @param $parsedBody + */ + public function assertBodyIsParsedProperly($method, $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, + OperationParams::class, + $index, + Utils::printSafe($entry) + )); + } + $errors = $entry->validate(); + + if (!empty($errors[0])) { + $err = $index ? "Error in query #$index: {$errors[0]}" : $errors[0]; + throw new InvariantViolation($err); + } + } } - return $op; + if ($parsedBody instanceof OperationParams) { + $errors = $parsedBody->validate(); + if (!empty($errors[0])) { + throw new InvariantViolation($errors[0]); + } + } + + 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 2a187ca..e81982d 100644 --- a/src/Server/OperationParams.php +++ b/src/Server/OperationParams.php @@ -1,6 +1,7 @@ query && $this->queryId) { - $errors[] = 'GraphQL Request parameters: "query" and "queryId" are mutually exclusive'; + $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::printSafe($this->query); + $errors[] = 'GraphQL Request parameter "query" must be string, but got ' . + Utils::printSafeJson($this->query); } - if ($this->queryId !== null && (!is_string($this->query) || empty($this->query))) { - $errors[] = 'GraphQL Request parameter "queryId" must be string, but got: ' . - Utils::printSafe($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::printSafe($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 associative array, but got: ' . - Utils::printSafe($this->variables); + $errors[] = 'GraphQL Request parameter "variables" must be object, but got ' . + Utils::printSafeJson($this->variables); } return $errors; } diff --git a/src/Server/StandardServer.php b/src/Server/StandardServer.php index d8aebff..632dd0f 100644 --- a/src/Server/StandardServer.php +++ b/src/Server/StandardServer.php @@ -32,6 +32,11 @@ class StandardServer */ private $config; + /** + * @var Helper + */ + private $helper; + /** * StandardServer constructor. * @param ServerConfig $config @@ -39,6 +44,7 @@ class StandardServer protected function __construct(ServerConfig $config) { $this->config = $config; + $this->helper = new Helper(); } /** @@ -48,59 +54,18 @@ class StandardServer public function executeRequest($parsedBody = null) { if (null !== $parsedBody) { - $this->assertBodyIsParsedProperly(__METHOD__, $parsedBody); + $this->helper->assertBodyIsParsedProperly(__METHOD__, $parsedBody); } else { - $parsedBody = Helper::parseHttpRequest(); + $parsedBody = $this->helper->parseHttpRequest(); } $batched = is_array($parsedBody); $result = []; foreach ((array) $parsedBody as $index => $operationParams) { - $result[] = Helper::executeOperation($this->config, $operationParams); + $result[] = $this->helper->executeOperation($this->config, $operationParams); } return $batched ? $result : $result[0]; } - - /** - * @param $method - * @param $parsedBody - */ - private function assertBodyIsParsedProperly($method, $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, - OperationParams::class, - $index, - Utils::printSafe($entry) - )); - } - $errors = $entry->validate(); - - if (!empty($errors[0])) { - $err = $index ? "Error in query #$index: {$errors[0]}" : $errors[0]; - throw new InvariantViolation($err); - } - } - } - - if ($parsedBody instanceof OperationParams) { - $errors = $parsedBody->validate(); - if (!empty($errors[0])) { - throw new InvariantViolation($errors[0]); - } - } - - 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/Utils/Utils.php b/src/Utils/Utils.php index 6de5f05..96e54fb 100644 --- a/src/Utils/Utils.php +++ b/src/Utils/Utils.php @@ -241,6 +241,48 @@ class Utils return is_object($var) ? get_class($var) : gettype($var); } + /** + * @param mixed $var + * @return string + */ + public static function printSafeJson($var) + { + if ($var instanceof \stdClass) { + $var = (array) $var; + } + if (is_array($var)) { + $count = count($var); + if (!isset($var[0]) && $count > 0) { + $keys = []; + $keyCount = 0; + foreach ($var as $key => $value) { + $keys[] = '"' . $key . '"'; + if ($keyCount++ > 4) { + break; + } + } + $keysLabel = $keyCount === 1 ? 'key' : 'keys'; + $msg = "object with first $keysLabel: " . implode(', ', $keys); + } else { + $msg = "array($count)"; + } + return $msg; + } + if ('' === $var) { + return '(empty string)'; + } + if (is_string($var)) { + return "\"$var\""; + } + if (is_scalar($var)) { + return (string) $var; + } + if (null === $var) { + return 'null'; + } + return gettype($var); + } + /** * @param $var * @return string @@ -259,12 +301,13 @@ class Utils $keys = []; $keyCount = 0; foreach ($var as $key => $value) { - $keys[] = $key; + $keys[] = '"' . $key . '"'; if ($keyCount++ > 4) { break; } } - $msg = "associative array($count) with first keys: " . implode(', ', $keys); + $keysLabel = $keyCount === 1 ? 'key' : 'keys'; + $msg = "associative array($count) with first $keysLabel: " . implode(', ', $keys); } else { $msg = "array($count)"; } diff --git a/tests/Server/QueryExecutionTest.php b/tests/Server/QueryExecutionTest.php index 092a97c..876d500 100644 --- a/tests/Server/QueryExecutionTest.php +++ b/tests/Server/QueryExecutionTest.php @@ -356,7 +356,8 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase private function executePersistedQuery($queryId, $variables = null) { $op = OperationParams::create(['queryId' => $queryId, 'variables' => $variables]); - $result = Helper::executeOperation($this->config, $op); + $helper = new Helper(); + $result = $helper->executeOperation($this->config, $op); $this->assertInstanceOf(ExecutionResult::class, $result); return $result; } @@ -364,8 +365,8 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase private function executeQuery($query, $variables = null) { $op = OperationParams::create(['query' => $query, 'variables' => $variables]); - - $result = Helper::executeOperation($this->config, $op); + $helper = new Helper(); + $result = $helper->executeOperation($this->config, $op); $this->assertInstanceOf(ExecutionResult::class, $result); return $result; } diff --git a/tests/Server/RequestParsingTest.php b/tests/Server/RequestParsingTest.php new file mode 100644 index 0000000..62ad5a1 --- /dev/null +++ b/tests/Server/RequestParsingTest.php @@ -0,0 +1,350 @@ +parseRawRequest('application/graphql', $query); + + $this->assertSame(['query' => $query], $parsedBody); + $this->assertFalse($isReadonly); + } + + public function testParsesSimpleUrlencodedRequest() + { + $query = '{my query}'; + $variables = ['test' => 1, 'test2' => 2]; + $operation = 'op'; + + $post = [ + 'query' => $query, + 'variables' => $variables, + 'operation' => $operation + ]; + + list ($parsedBody, $isReadonly) = $this->parseFormUrlencodedRequest($post); + $this->assertSame($post, $parsedBody); + $this->assertFalse($isReadonly); + } + + public function testParsesSimpleGETRequest() + { + $query = '{my query}'; + $variables = ['test' => 1, 'test2' => 2]; + $operation = 'op'; + + $get = [ + 'query' => $query, + 'variables' => $variables, + 'operation' => $operation + ]; + + list ($parsedBody, $isReadonly) = $this->parseGetRequest($get); + $this->assertSame($get, $parsedBody); + $this->assertTrue($isReadonly); + } + + public function testParsesSimpleJSONRequest() + { + $query = '{my query}'; + $variables = ['test' => 1, 'test2' => 2]; + $operation = 'op'; + + $body = [ + 'query' => $query, + 'variables' => $variables, + 'operation' => $operation + ]; + + list ($parsedBody, $isReadonly) = $this->parseRawRequest('application/json', json_encode($body)); + $this->assertEquals($body, $parsedBody); + $this->assertFalse($isReadonly); + } + + public function testParsesBatchJSONRequest() + { + $body = [ + [ + 'query' => '{my query}', + 'variables' => ['test' => 1, 'test2' => 2], + 'operation' => 'op' + ], + [ + 'queryId' => 'my-query-id', + 'variables' => ['test' => 1, 'test2' => 2], + 'operation' => 'op2' + ], + ]; + + list ($parsedBody, $isReadonly) = $this->parseRawRequest('application/json', json_encode($body)); + $this->assertEquals($body, $parsedBody); + $this->assertFalse($isReadonly); + } + + public function testFailsParsingInvalidJsonRequest() + { + $body = 'not really{} a json'; + + $this->setExpectedException(UserError::class, 'Could not parse JSON: Syntax error'); + $this->parseRawRequest('application/json', $body); + } + + public function testFailsParsingNonArrayOrObjectJsonRequest() + { + $body = '"str"'; + + $this->setExpectedException(UserError::class, 'GraphQL Server expects JSON object or array, but got "str"'); + $this->parseRawRequest('application/json', $body); + } + + public function testFailsParsingInvalidGetRequest() + { + $this->setExpectedException(UserError::class, 'Cannot execute GET request without "query" or "queryId" parameter'); + $this->parseGetRequest([]); + } + + public function testFailsParsingInvalidContentType() + { + $this->setExpectedException(UserError::class, 'Unexpected content type: "not-supported-content-type"'); + $this->parseRawRequest('not-supported-content-type', 'test'); + } + + public function testFailsWithMissingContentType() + { + $this->setExpectedException(UserError::class, 'Missing "Content-Type" header'); + $this->parseRawRequest(null, 'test'); + } + + public function testFailsOnMethodsOtherThanPostOrGet() + { + $this->setExpectedException(UserError::class, 'HTTP Method "PUT" is not supported'); + $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 + * @param $method + * + * @return OperationParams|OperationParams[] + */ + private function parseRawRequest($contentType, $content, $method = 'POST') + { + $_SERVER['CONTENT_TYPE'] = $contentType; + $_SERVER['REQUEST_METHOD'] = $method; + + $helper = new Helper(); + return $helper->parseRawBody(function() use ($content) { + return $content; + }); + } + + /** + * @param array $postValue + * @return OperationParams|OperationParams[] + */ + private function parseFormUrlencodedRequest($postValue) + { + $_SERVER['CONTENT_TYPE'] = 'application/x-www-form-urlencoded'; + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_POST = $postValue; + + $helper = new Helper(); + return $helper->parseRawBody(); + } + + /** + * @param $getValue + * @return array + */ + private function parseGetRequest($getValue) + { + $_SERVER['REQUEST_METHOD'] = 'GET'; + $_GET = $getValue; + + $helper = new Helper(); + return $helper->parseRawBody(); + } + + /** + * @param OperationParams $params + * @param string $query + * @param string $queryId + * @param array $variables + * @param string $operation + */ + private function assertValidOperationParams($params, $query, $queryId = null, $variables = null, $operation = null) + { + $this->assertInstanceOf(OperationParams::class, $params); + + $this->assertSame($query, $params->query); + $this->assertSame($queryId, $params->queryId); + $this->assertSame($variables, $params->variables); + $this->assertSame($operation, $params->operation); + } +}