Server: throw only when there is a configuration or logic error (invariant violation)

This commit is contained in:
Vladimir Razuvaev 2017-07-19 19:30:39 +07:00
parent 90088c7bde
commit e6e531b88b
8 changed files with 241 additions and 242 deletions

View File

@ -4,7 +4,6 @@ namespace GraphQL\Server;
use GraphQL\Error\Error;
use GraphQL\Error\FormattedError;
use GraphQL\Error\InvariantViolation;
use GraphQL\Error\UserError;
use GraphQL\Executor\ExecutionResult;
use GraphQL\Executor\Executor;
use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter;
@ -36,7 +35,14 @@ class Helper
public function executeOperation(ServerConfig $config, OperationParams $op)
{
$promiseAdapter = $config->getPromiseAdapter() ?: Executor::getPromiseAdapter();
$errors = $this->validateOperationParams($op);
if (!empty($errors)) {
$result = $promiseAdapter->createFulfilled(new ExecutionResult(null, $errors));
} else {
$result = $this->promiseToExecuteOperation($promiseAdapter, $config, $op);
}
if ($promiseAdapter instanceof SyncPromiseAdapter) {
$result = $promiseAdapter->wait($result);
@ -59,8 +65,13 @@ class Helper
$result = [];
foreach ($operations as $operation) {
$errors = $this->validateOperationParams($operation);
if (!empty($errors)) {
$result[] = $promiseAdapter->createFulfilled(new ExecutionResult(null, $errors));
} else {
$result[] = $this->promiseToExecuteOperation($promiseAdapter, $config, $operation);
}
}
$result = $promiseAdapter->all($result);
@ -79,17 +90,14 @@ class Helper
*/
private function promiseToExecuteOperation(PromiseAdapter $promiseAdapter, ServerConfig $config, OperationParams $op)
{
$phpErrors = [];
$execute = function() use ($config, $op, $promiseAdapter) {
try {
$doc = $op->queryId ? static::loadPersistedQuery($config, $op) : $op->query;
if (!$doc instanceof DocumentNode) {
$doc = Parser::parse($doc);
}
if ($op->isReadOnly() && AST::isMutation($op->operation, $doc)) {
throw new UserError("Cannot execute mutation in read-only context");
if ($op->isReadOnly() && AST::getOperation($doc, $op->operation) !== 'query') {
throw new Error("GET supports only query operation");
}
$validationErrors = DocumentValidator::validate(
@ -99,11 +107,11 @@ class Helper
);
if (!empty($validationErrors)) {
return $promiseAdapter->createFulfilled(
$result = $promiseAdapter->createFulfilled(
new ExecutionResult(null, $validationErrors)
);
} else {
return Executor::promiseToExecute(
$result = Executor::promiseToExecute(
$promiseAdapter,
$config->getSchema(),
$doc,
@ -115,17 +123,12 @@ class Helper
);
}
} catch (Error $e) {
return $promiseAdapter->createFulfilled(
$result = $promiseAdapter->createFulfilled(
new ExecutionResult(null, [$e])
);
}
};
if ($config->getDebug()) {
$execute = Utils::withErrorHandling($execute, $phpErrors);
}
$result = $execute();
$applyErrorFormatting = function (ExecutionResult $result) use ($config, $phpErrors) {
$applyErrorFormatting = function (ExecutionResult $result) use ($config) {
if ($config->getDebug()) {
$errorFormatter = function($e) {
return FormattedError::createFromException($e, true);
@ -133,9 +136,6 @@ class Helper
} else {
$errorFormatter = $config->getErrorFormatter();
}
if (!empty($phpErrors)) {
$result->extensions['phpErrors'] = array_map($errorFormatter, $phpErrors);
}
$result->setErrorFormatter($errorFormatter);
return $result;
};
@ -160,7 +160,7 @@ class Helper
$loader = $config->getPersistentQueryLoader();
if (!$loader) {
throw new UserError("Persisted queries are not supported by this server");
throw new Error("Persisted queries are not supported by this server");
}
$source = $loader($op->queryId, $op);
@ -190,10 +190,10 @@ class Helper
$validationRules = $validationRules($params);
if (!is_array($validationRules)) {
throw new InvariantViolation(
"Validation rules callable must return array of rules, but got: %s" .
throw new InvariantViolation(sprintf(
"Expecting validation rules to be array or callable returning array, but got: %s",
Utils::printSafe($validationRules)
);
));
}
}
@ -210,19 +210,14 @@ class Helper
*
* @param callable|null $readRawBodyFn
* @return OperationParams|OperationParams[]
* @throws Error
*/
public function parseHttpRequest(callable $readRawBodyFn = null)
{
$method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : null;
if ($method === 'GET') {
$request = array_change_key_case($_GET);
if (isset($request['query']) || isset($request['queryid']) || isset($request['documentid'])) {
$result = OperationParams::create($_GET, true);
} else {
throw new UserError('Cannot execute GET request without "query" or "queryId" parameter');
}
} else if ($method === 'POST') {
$contentType = isset($_SERVER['CONTENT_TYPE']) ? $_SERVER['CONTENT_TYPE'] : null;
@ -234,10 +229,10 @@ class Helper
$body = json_decode($rawBody ?: '', true);
if (json_last_error()) {
throw new UserError("Could not parse JSON: " . json_last_error_msg());
throw new Error("Could not parse JSON: " . json_last_error_msg());
}
if (!is_array($body)) {
throw new UserError(
throw new Error(
"GraphQL Server expects JSON object or array, but got " .
Utils::printSafeJson($body)
);
@ -245,7 +240,7 @@ class Helper
if (isset($body[0])) {
$result = [];
foreach ($body as $index => $entry) {
$op = OperationParams::create($entry, true);
$op = OperationParams::create($entry);
$result[] = $op;
}
} else {
@ -254,12 +249,12 @@ class Helper
} else if (stripos($contentType, 'application/x-www-form-urlencoded') !== false) {
$result = OperationParams::create($_POST);
} else if (null === $contentType) {
throw new UserError('Missing "Content-Type" header');
throw new Error('Missing "Content-Type" header');
} else {
throw new UserError("Unexpected content type: " . Utils::printSafeJson($contentType));
throw new Error("Unexpected content type: " . Utils::printSafeJson($contentType));
}
} else {
throw new UserError('HTTP Method "' . $method . '" is not supported', 405);
throw new Error('HTTP Method "' . $method . '" is not supported');
}
return $result;
}
@ -276,77 +271,43 @@ class Helper
* Checks validity of operation params and returns array of errors (empty array when params are valid)
*
* @param OperationParams $params
* @return array
* @return Error[]
*/
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"';
$errors[] = new Error('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';
$errors[] = new Error('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);
$errors[] = new Error(
'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);
$errors[] = new Error(
'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);
$errors[] = new Error(
'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);
$errors[] = new Error(
'GraphQL Request parameter "variables" must be object or JSON string parsed to object, but got ' .
Utils::printSafeJson($params->getOriginalInput('variables'))
);
}
return $errors;
}
/**
* Assertion to check that parsed body is valid instance of OperationParams (or array of instances)
*
* @param OperationParams|OperationParams[] $parsedBody
* @throws InvariantViolation
* @throws UserError
*/
public function assertValidRequest($parsedBody)
{
if (is_array($parsedBody)) {
foreach ($parsedBody as $index => $entry) {
if (!$entry instanceof OperationParams) {
throw new InvariantViolation(sprintf(
'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 = $this->validateOperationParams($entry);
if (!empty($errors[0])) {
$err = $index ? "Error in query #$index: {$errors[0]}" : $errors[0];
throw new UserError($err);
}
}
} else if ($parsedBody instanceof OperationParams) {
$errors = $this->validateOperationParams($parsedBody);
if (!empty($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)
));
}
}
}

View File

@ -1,8 +1,6 @@
<?php
namespace GraphQL\Server;
use GraphQL\Utils\Utils;
/**
* Class QueryParams
* Represents all available parsed query parameters
@ -52,9 +50,9 @@ class OperationParams
public static function create(array $params, $readonly = false)
{
$instance = new static();
$instance->originalInput = $params;
$params = array_change_key_case($params, CASE_LOWER);
$instance->originalInput = $params;
$params += [
'query' => null,
@ -64,6 +62,13 @@ class OperationParams
'variables' => null
];
if (is_string($params['variables'])) {
$tmp = json_decode($params['variables'], true);
if (!json_last_error()) {
$params['variables'] = $tmp;
}
}
$instance->query = $params['query'];
$instance->queryId = $params['queryid'] ?: $params['documentid'];
$instance->operation = $params['operation'];
@ -74,11 +79,12 @@ class OperationParams
}
/**
* @return array
* @param string $key
* @return mixed
*/
public function getOriginalInput()
public function getOriginalInput($key)
{
return $this->originalInput;
return isset($this->originalInput[$key]) ? $this->originalInput[$key] : null;
}
/**

View File

@ -1,6 +1,7 @@
<?php
namespace GraphQL\Server;
use GraphQL\Error\InvariantViolation;
use GraphQL\Executor\ExecutionResult;
use GraphQL\Executor\Promise\Promise;
@ -48,13 +49,13 @@ class StandardServer
/**
* @param OperationParams|OperationParams[] $parsedBody
* @return ExecutionResult|ExecutionResult[]|Promise
* @throws InvariantViolation
*/
public function executeRequest($parsedBody = null)
{
if (null !== $parsedBody) {
$parsedBody = $this->helper->parseHttpRequest();
}
$this->helper->assertValidRequest($parsedBody);
if (is_array($parsedBody)) {
return $this->helper->executeBatch($this->config, $parsedBody);

View File

@ -336,17 +336,17 @@ class AST
}
/**
* @param string $operation
* @param DocumentNode $document
* @param string $operationName
* @return bool
*/
public static function isMutation($operation, DocumentNode $document)
public static function getOperation(DocumentNode $document, $operationName = null)
{
if (is_array($document->definitions)) {
foreach ($document->definitions as $def) {
if ($def instanceof OperationDefinitionNode) {
if ($def->operation === 'mutation' && $def->name->value === $operation) {
return true;
if (!$operationName || (isset($def->name->value) && $def->name->value === $operationName)) {
return $def->operation;
}
}
}

View File

@ -271,15 +271,21 @@ class Utils
if ('' === $var) {
return '(empty string)';
}
if (null === $var) {
return 'null';
}
if (false === $var) {
return 'false';
}
if (true === $var) {
return 'false';
}
if (is_string($var)) {
return "\"$var\"";
}
if (is_scalar($var)) {
return (string) $var;
}
if (null === $var) {
return 'null';
}
return gettype($var);
}
@ -316,15 +322,21 @@ class Utils
if ('' === $var) {
return '(empty string)';
}
if (null === $var) {
return 'null';
}
if (false === $var) {
return 'false';
}
if (true === $var) {
return 'false';
}
if (is_string($var)) {
return "\"$var\"";
}
if (is_scalar($var)) {
return (string) $var;
}
if (null === $var) {
return 'null';
}
return gettype($var);
}

View File

@ -3,6 +3,7 @@ namespace GraphQL\Tests\Server;
use GraphQL\Deferred;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Error\UserError;
use GraphQL\Executor\ExecutionResult;
use GraphQL\Language\Parser;
@ -85,6 +86,19 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
}
]
]
]),
'mutation' => new ObjectType([
'name' => 'Mutation',
'fields' => [
'm1' => [
'type' => new ObjectType([
'name' => 'TestMutation',
'fields' => [
'result' => Type::string()
]
])
]
]
])
]);
@ -104,39 +118,17 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
$this->assertQueryResultEquals($expected, $query);
}
public function testDebugPhpErrors()
public function testReturnsSyntaxErrors()
{
$this->config->setDebug(true);
$query = '{f1';
$query = '
{
fieldWithPhpError
f1
}
';
$expected = [
'data' => [
'fieldWithPhpError' => 'fieldWithPhpError',
'f1' => 'f1'
],
'extensions' => [
'phpErrors' => [
['debugMessage' => 'deprecated', 'severity' => 16384],
['debugMessage' => 'notice', 'severity' => 1024],
['debugMessage' => 'warning', 'severity' => 512],
['debugMessage' => 'Undefined index: test', 'severity' => 8],
]
]
];
$result = $this->assertQueryResultEquals($expected, $query);
// Assert php errors contain trace:
$this->assertArrayHasKey('trace', $result->extensions['phpErrors'][0]);
$this->assertArrayHasKey('trace', $result->extensions['phpErrors'][1]);
$this->assertArrayHasKey('trace', $result->extensions['phpErrors'][2]);
$this->assertArrayHasKey('trace', $result->extensions['phpErrors'][3]);
$result = $this->executeQuery($query);
$this->assertSame(null, $result->data);
$this->assertCount(1, $result->errors);
$this->assertContains(
'Syntax Error GraphQL (1:4) Expected Name, found <EOF>',
$result->errors[0]->getMessage()
);
}
public function testDebugExceptions()
@ -280,8 +272,32 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
public function testPersistedQueriesAreDisabledByDefault()
{
$this->setExpectedException(UserError::class, 'Persisted queries are not supported by this server');
$this->executePersistedQuery('some-id');
$result = $this->executePersistedQuery('some-id');
$expected = [
'errors' => [
[
'message' => 'Persisted queries are not supported by this server'
]
]
];
$this->assertEquals($expected, $result->toArray());
}
public function testMutationsAreNotAllowedInReadonlyMode()
{
$mutation = 'mutation { a }';
$expected = [
'errors' => [
[
'message' => 'GET supports only query operation'
]
]
];
$result = $this->executeQuery($mutation, null, true);
$this->assertEquals($expected, $result->toArray());
}
public function testAllowsPersistentQueries()
@ -315,6 +331,19 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($expected, $result->toArray());
}
public function testProhibitsInvalidPersistedQueryLoader()
{
$this->setExpectedException(
InvariantViolation::class,
'Persistent query loader must return query string or instance of GraphQL\Language\AST\DocumentNode '.
'but got: associative array(1) with first key: "err"'
);
$this->config->setPersistentQueryLoader(function($queryId, OperationParams $params) use (&$called) {
return ['err' => 'err'];
});
$this->executePersistedQuery('some-id');
}
public function testPersistedQueriesAreStillValidatedByDefault()
{
$this->config->setPersistentQueryLoader(function() {
@ -369,6 +398,18 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($expected, $result->toArray());
}
public function testProhibitsUnexpectedValidationRules()
{
$this->setExpectedException(
InvariantViolation::class,
'Expecting validation rules to be array or callable returning array, but got: instance of stdClass'
);
$this->config->setValidationRules(function(OperationParams $params) {
return new \stdClass();
});
$this->executeQuery('{f1}');
}
public function testExecutesBatchedQueries()
{
$batch = [
@ -489,9 +530,9 @@ class QueryExecutionTest extends \PHPUnit_Framework_TestCase
return $result;
}
private function executeQuery($query, $variables = null)
private function executeQuery($query, $variables = null, $readonly = false)
{
$op = OperationParams::create(['query' => $query, 'variables' => $variables]);
$op = OperationParams::create(['query' => $query, 'variables' => $variables], $readonly);
$helper = new Helper();
$result = $helper->executeOperation($this->config, $op);
$this->assertInstanceOf(ExecutionResult::class, $result);

View File

@ -1,8 +1,8 @@
<?php
namespace GraphQL\Tests\Server;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Error\UserError;
use GraphQL\Server\Helper;
use GraphQL\Server\OperationParams;
@ -67,6 +67,38 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase
$this->assertFalse($parsedBody->isReadOnly());
}
public function testParsesVariablesAsJSON()
{
$query = '{my query}';
$variables = ['test' => 1, 'test2' => 2];
$operation = 'op';
$body = [
'query' => $query,
'variables' => json_encode($variables),
'operation' => $operation
];
$parsedBody = $this->parseRawRequest('application/json', json_encode($body));
$this->assertValidOperationParams($parsedBody, $query, null, $variables, $operation);
$this->assertFalse($parsedBody->isReadOnly());
}
public function testIgnoresInvalidVariablesJson()
{
$query = '{my query}';
$variables = '"some invalid json';
$operation = 'op';
$body = [
'query' => $query,
'variables' => $variables,
'operation' => $operation
];
$parsedBody = $this->parseRawRequest('application/json', json_encode($body));
$this->assertValidOperationParams($parsedBody, $query, null, $variables, $operation);
$this->assertFalse($parsedBody->isReadOnly());
}
public function testParsesBatchJSONRequest()
{
$body = [
@ -88,17 +120,13 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase
$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()
{
$body = 'not really{} a json';
$this->setExpectedException(UserError::class, 'Could not parse JSON: Syntax error');
$this->setExpectedException(Error::class, 'Could not parse JSON: Syntax error');
$this->parseRawRequest('application/json', $body);
}
@ -106,31 +134,25 @@ class RequestParsingTest extends \PHPUnit_Framework_TestCase
{
$body = '"str"';
$this->setExpectedException(UserError::class, 'GraphQL Server expects JSON object or array, but got "str"');
$this->setExpectedException(Error::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->setExpectedException(Error::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->setExpectedException(Error::class, 'Missing "Content-Type" header');
$this->parseRawRequest(null, 'test');
}
public function testFailsOnMethodsOtherThanPostOrGet()
{
$this->setExpectedException(UserError::class, 'HTTP Method "PUT" is not supported');
$this->setExpectedException(Error::class, 'HTTP Method "PUT" is not supported');
$this->parseRawRequest(null, 'test', "PUT");
}

View File

@ -1,8 +1,6 @@
<?php
namespace GraphQL\Tests\Server;
use GraphQL\Error\InvariantViolation;
use GraphQL\Error\UserError;
use GraphQL\Server\Helper;
use GraphQL\Server\OperationParams;
@ -38,44 +36,6 @@ class RequestValidationTest extends \PHPUnit_Framework_TestCase
$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([
@ -143,37 +103,33 @@ class RequestValidationTest extends \PHPUnit_Framework_TestCase
{
$parsedBody = OperationParams::create([
'query' => '{my query}',
'variables' => 'test'
'variables' => 0
]);
$this->assertInputError($parsedBody, 'GraphQL Request parameter "variables" must be object, but got "test"');
$this->assertInputError(
$parsedBody,
'GraphQL Request parameter "variables" must be object or JSON string parsed to object, but got 0'
);
}
private function assertValid($parsedRequest)
{
$helper = new Helper();
$helper->assertValidRequest($parsedRequest);
$errors = $helper->validateOperationParams($parsedRequest);
if (!empty($errors)) {
throw $errors[0];
}
}
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());
$errors = $helper->validateOperationParams($parsedRequest);
if (!empty($errors[0])) {
$this->assertEquals($expectedMessage, $errors[0]->getMessage());
} else {
$this->fail('Expected error not returned');
}
}
}