Fix unhandled error when parsing custom scalar literals.

This factors out the enum value validation from scalar value validation and ensures the same try/catch is used in isValidLiteralValue as isValidPHPValue and protecting from errors in valueFromAST.
ref: graphql/graphql-js#1126
This commit is contained in:
Daniel Tschinder 2018-02-11 22:31:04 +01:00
parent 481cdc9a82
commit f661f38215
9 changed files with 155 additions and 114 deletions

View File

@ -936,7 +936,12 @@ const UNION_TYPE_DEFINITION = "UnionTypeDefinition";
const ENUM_TYPE_DEFINITION = "EnumTypeDefinition";
const ENUM_VALUE_DEFINITION = "EnumValueDefinition";
const INPUT_OBJECT_TYPE_DEFINITION = "InputObjectTypeDefinition";
const SCALAR_TYPE_EXTENSION = "ScalarTypeExtension";
const OBJECT_TYPE_EXTENSION = "ObjectTypeExtension";
const INTERFACE_TYPE_EXTENSION = "InterfaceTypeExtension";
const UNION_TYPE_EXTENSION = "UnionTypeExtension";
const ENUM_TYPE_EXTENSION = "EnumTypeExtension";
const INPUT_OBJECT_TYPE_EXTENSION = "InputObjectTypeExtension";
const DIRECTIVE_DEFINITION = "DirectiveDefinition";
```

View File

@ -273,31 +273,36 @@ class Values
return $errors;
}
Utils::invariant($type instanceof EnumType || $type instanceof ScalarType, 'Must be input type');
try {
// Scalar/Enum input checks to ensure the type can parse the value to
// a non-null value.
if (!$type->isValidValue($value)) {
$v = Utils::printSafeJson($value);
return [
"Expected type \"{$type->name}\", found $v."
];
if ($type instanceof EnumType) {
if (!is_string($value) || !$type->getValue($value)) {
$printed = Utils::printSafeJson($value);
return ["Expected type \"{$type->name}\", found $printed."];
}
} catch (\Exception $e) {
return [
"Expected type \"{$type->name}\", found " . Utils::printSafeJson($value) . ': ' .
$e->getMessage()
];
} catch (\Throwable $e) {
return [
"Expected type \"{$type->name}\", found " . Utils::printSafeJson($value) . ': ' .
$e->getMessage()
];
return [];
}
Utils::invariant($type instanceof ScalarType, 'Must be a scalar type');
/** @var ScalarType $type */
// Scalars determine if a value is valid via parseValue().
try {
$parseResult = $type->parseValue($value);
if (Utils::isInvalid($parseResult)) {
$printed = Utils::printSafeJson($value);
return [
"Expected type \"{$type->name}\", found $printed."
];
}
} catch (\Exception $error) {
$printed = Utils::printSafeJson($value);
$message = $error->getMessage();
return ["Expected type \"{$type->name}\", found $printed; $message"];
} catch (\Throwable $error) {
$printed = Utils::printSafeJson($value);
$message = $error->getMessage();
return ["Expected type \"{$type->name}\", found $printed; $message"];
}
return [];
}
@ -370,12 +375,34 @@ class Values
return $coercedObj;
}
Utils::invariant($type instanceof EnumType || $type instanceof ScalarType, 'Must be input type');
if ($type instanceof EnumType) {
if (!is_string($value) || !$type->getValue($value)) {
return $undefined;
}
if ($type->isValidValue($value)) {
return $type->parseValue($value);
$enumValue = $type->getValue($value);
if (!$enumValue) {
return $undefined;
}
return $enumValue->value;
}
return $undefined;
Utils::invariant($type instanceof ScalarType, 'Must be a scalar type');
/** @var ScalarType $type */
// Scalars determine if a value is valid via parseValue().
try {
$parseResult = $type->parseValue($value);
if (Utils::isInvalid($parseResult)) {
return $undefined;
}
} catch (\Exception $error) {
return $undefined;
} catch (\Throwable $error) {
return $undefined;
}
return $parseResult;
}
}

View File

@ -115,25 +115,6 @@ class EnumType extends Type implements InputType, OutputType, LeafType
return Utils::undefined();
}
/**
* @param string $value
* @return bool
*/
public function isValidValue($value)
{
return is_string($value) && $this->getNameLookup()->offsetExists($value);
}
/**
* @param $valueNode
* @param array|null $variables
* @return bool
*/
public function isValidLiteral($valueNode, array $variables = null)
{
return $valueNode instanceof EnumValueNode && $this->getNameLookup()->offsetExists($valueNode->value);
}
/**
* @param $value
* @return null

View File

@ -38,17 +38,4 @@ interface LeafType
* @return mixed
*/
public function parseLiteral($valueNode, array $variables = null);
/**
* @param string $value
* @return bool
*/
public function isValidValue($value);
/**
* @param Node $valueNode
* @param array|null $variables
* @return bool
*/
public function isValidLiteral($valueNode, array $variables = null);
}

View File

@ -38,28 +38,4 @@ abstract class ScalarType extends Type implements OutputType, InputType, LeafTyp
Utils::assertValidName($this->name);
}
/**
* Determines if an internal value is valid for this type.
*
* @param $value
* @return bool
*/
public function isValidValue($value)
{
return !Utils::isInvalid($this->parseValue($value));
}
/**
* Determines if an internal value is valid for this type.
* Equivalent to checking for if the parsedLiteral is nullish.
*
* @param $valueNode
* @param array|null $variables
* @return bool
*/
public function isValidLiteral($valueNode, array $variables = null)
{
return !Utils::isInvalid($this->parseLiteral($valueNode, $variables));
}
}

View File

@ -383,15 +383,37 @@ class AST
return $coercedObj;
}
if (!$type instanceof ScalarType && !$type instanceof EnumType) {
throw new InvariantViolation('Must be input type');
if ($type instanceof EnumType) {
if (!$valueNode instanceof EnumValueNode) {
return $undefined;
}
$enumValue = $type->getValue($valueNode->value);
if (!$enumValue) {
return $undefined;
}
return $enumValue->value;
}
if ($type->isValidLiteral($valueNode, $variables)) {
return $type->parseLiteral($valueNode, $variables);
Utils::invariant($type instanceof ScalarType, 'Must be scalar type');
/** @var ScalarType $type */
// Scalars fulfill parsing a literal value via parseLiteral().
// Invalid values represent a failure to parse correctly, in which case
// no value is returned.
try {
$result = $type->parseLiteral($valueNode, $variables);
} catch (\Exception $error) {
return $undefined;
} catch (\Throwable $error) {
return $undefined;
}
return $undefined;
if (Utils::isInvalid($result)) {
return $undefined;
}
return $result;
}
/**

View File

@ -2,6 +2,7 @@
namespace GraphQL\Validator;
use GraphQL\Error\Error;
use GraphQL\Language\AST\EnumValueNode;
use GraphQL\Language\AST\ListValueNode;
use GraphQL\Language\AST\DocumentNode;
use GraphQL\Language\AST\NodeKind;
@ -309,12 +310,33 @@ class DocumentValidator
return $errors;
}
Utils::invariant($type instanceof ScalarType || $type instanceof EnumType, 'Must be input type');
if ($type instanceof EnumType) {
if (!$valueNode instanceof EnumValueNode || !$type->getValue($valueNode->value)) {
$printed = Printer::doPrint($valueNode);
return ["Expected type \"{$type->name}\", found $printed."];
}
// Scalars determine if a literal values is valid.
if (!$type->isValidLiteral($valueNode)) {
return [];
}
Utils::invariant($type instanceof ScalarType, 'Must be a scalar type');
/** @var ScalarType $type */
// Scalars determine if a literal values is valid via parseLiteral().
try {
$parseResult = $type->parseLiteral($valueNode);
if (Utils::isInvalid($parseResult)) {
$printed = Printer::doPrint($valueNode);
return ["Expected type \"{$type->name}\", found $printed."];
}
} catch (\Exception $error) {
$printed = Printer::doPrint($valueNode);
return [ "Expected type \"{$type->name}\", found $printed." ];
$message = $error->getMessage();
return ["Expected type \"{$type->name}\", found $printed; $message"];
} catch (\Throwable $error) {
$printed = Printer::doPrint($valueNode);
$message = $error->getMessage();
return ["Expected type \"{$type->name}\", found $printed; $message"];
}
return [];

View File

@ -260,17 +260,6 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
]
]);
$invalidScalar = new CustomScalarType([
'name' => 'Invalid',
'serialize' => function ($value) { return $value; },
'parseLiteral' => function ($node) {
throw new \Exception('Invalid scalar is always invalid: ' . $node->value);
},
'parseValue' => function ($value) {
throw new \Exception('Invalid scalar is always invalid: ' . $value);
},
]);
$anyScalar = new CustomScalarType([
'name' => 'Any',
'serialize' => function ($value) { return $value; },
@ -278,6 +267,19 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
'parseValue' => function ($value) { return $value; }, // Allows any value
]);
$invalidScalar = new CustomScalarType([
'name' => 'Invalid',
'serialize' => function ($value) {
return $value;
},
'parseLiteral' => function ($node) {
throw new \Exception('Invalid scalar is always invalid: ' . $node->value);
},
'parseValue' => function ($node) {
throw new \Exception('Invalid scalar is always invalid: ' . $node);
},
]);
$queryRoot = new ObjectType([
'name' => 'QueryRoot',
'fields' => [
@ -293,14 +295,16 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
'dogOrHuman' => ['type' => $DogOrHuman],
'humanOrAlien' => ['type' => $HumanOrAlien],
'complicatedArgs' => ['type' => $ComplicatedArgs],
'invalidArg' => [
'args' => ['arg' => ['type' => $invalidScalar]],
'type' => Type::string(),
],
'anyArg' => [
'args' => ['arg' => ['type' => $anyScalar]],
'type' => Type::string(),
],
'invalidArg' => [
'args' => [
'arg' => ['type' => $invalidScalar]
],
'type' => Type::string(),
]
]
]);

View File

@ -1,6 +1,7 @@
<?php
namespace GraphQL\Tests\Validator;
use GraphQL\Error\FormattedError;
use GraphQL\Validator\DocumentValidator;
use GraphQL\Validator\Rules\QueryComplexity;
@ -26,16 +27,32 @@ class ValidationTest extends TestCase
}
');
}
/*
public function testAllowsSettingRulesGlobally()
{
$rule = new QueryComplexity(0);
DocumentValidator::addRule($rule);
$instance = DocumentValidator::getRule(QueryComplexity::class);
$this->assertSame($rule, $instance);
/**
* @it detects bad scalar parse
*/
public function testDetectsBadScalarParse()
{
$doc = '
query {
invalidArg(arg: "bad value")
}
';
$expectedError = [
'message' => "Argument \"arg\" has invalid value \"bad value\".
Expected type \"Invalid\", found \"bad value\"; Invalid scalar is always invalid: bad value",
'locations' => [ ['line' => 3, 'column' => 25] ]
];
$this->expectInvalid(
$this->getTestSchema(),
null,
$doc,
[$expectedError]
);
}
*/
public function testPassesValidationWithEmptyRules()
{
$query = '{invalid}';