Skip to content

Commit f569c6d

Browse files
vladarvladar
committed
Spec compliance: coercion of Int values
1 parent 90e1ea4 commit f569c6d

File tree

5 files changed

+164
-21
lines changed

5 files changed

+164
-21
lines changed

‎src/Executor/Values.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ public static function valueFromAST($valueNode, InputType $type, $variables = nu
183183
* @param InputType $type
184184
* @return array
185185
*/
186-
private static function isValidPHPValue($value, InputType $type)
186+
public static function isValidPHPValue($value, InputType $type)
187187
{
188188
// A value must be provided if the type is non-null.
189189
if ($type instanceof NonNull) {

‎src/Type/Definition/IntType.php‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
<?php
22
namespace GraphQL\Type\Definition;
33

4+
use GraphQL\Error\InvariantViolation;
45
use GraphQL\Error\UserError;
56
use GraphQL\Language\AST\IntValueNode;
6-
use GraphQL\Language\AST\ValueNode;
77
use GraphQL\Utils;
88

99
/**
@@ -47,7 +47,11 @@ public function serialize($value)
4747
*/
4848
public function parseValue($value)
4949
{
50-
return $this->coerceInt($value);
50+
try {
51+
return $this->coerceInt($value);
52+
} catch (InvariantViolation $e) {
53+
throw new UserError($e->getMessage(), $e->getCode(), $e);
54+
}
5155
}
5256

5357
/**
@@ -57,19 +61,33 @@ public function parseValue($value)
5761
private function coerceInt($value)
5862
{
5963
if ($value === '') {
60-
throw new UserError(
64+
throw new InvariantViolation(
6165
'Int cannot represent non 32-bit signed integer value: (empty string)'
6266
);
6367
}
6468
if (false === $value || true === $value) {
6569
return (int) $value;
6670
}
67-
if (is_numeric($value) && $value <= self::MAX_INT && $value >= self::MIN_INT) {
68-
return (int) $value;
71+
if (!is_numeric($value) || $value > self::MAX_INT || $value < self::MIN_INT) {
72+
throw new InvariantViolation(
73+
sprintf('Int cannot represent non 32-bit signed integer value: %s', Utils::printSafe($value))
74+
);
75+
}
76+
$num = (float) $value;
77+
78+
// The GraphQL specification does not allow serializing non-integer values
79+
// as Int to avoid accidental data loss.
80+
// Examples: 1.0 == 1; 1.1 != 1, etc
81+
if ($num != (int)$value) {
82+
// Additionally account for scientific notation (i.e. 1e3), because (float)'1e3' is 1000, but (int)'1e3' is 1
83+
$trimmed = floor($num);
84+
if ($trimmed !== $num) {
85+
throw new InvariantViolation(
86+
'Int cannot represent non-integer value: ' . Utils::printSafe($value)
87+
);
88+
}
6989
}
70-
throw new UserError(
71-
sprintf('Int cannot represent non 32-bit signed integer value: %s', Utils::printSafe($value))
72-
);
90+
return (int) $value;
7391
}
7492

7593
/**

‎tests/Type/ScalarSerializationTest.php‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<?php
22
namespace GraphQL\Tests\Type;
33

4+
use GraphQL\Error\InvariantViolation;
45
use GraphQL\Error\UserError;
56
use GraphQL\Type\Definition\Type;
67

@@ -19,53 +20,77 @@ public function testSerializesOutputInt()
1920
$this->assertSame(123, $intType->serialize('123'));
2021
$this->assertSame(0, $intType->serialize(0));
2122
$this->assertSame(-1, $intType->serialize(-1));
22-
$this->assertSame(0, $intType->serialize(0.1));
23-
$this->assertSame(1, $intType->serialize(1.1));
24-
$this->assertSame(-1, $intType->serialize(-1.1));
2523
$this->assertSame(100000, $intType->serialize(1e5));
24+
$this->assertSame(0, $intType->serialize(0e5));
25+
26+
// The GraphQL specification does not allow serializing non-integer values
27+
// as Int to avoid accidental data loss.
28+
try {
29+
$intType->serialize(0.1);
30+
$this->fail('Expected exception not thrown');
31+
} catch (InvariantViolation $e) {
32+
$this->assertEquals('Int cannot represent non-integer value: 0.1', $e->getMessage());
33+
}
34+
try {
35+
$intType->serialize(1.1);
36+
$this->fail('Expected exception not thrown');
37+
} catch (InvariantViolation $e) {
38+
$this->assertEquals('Int cannot represent non-integer value: 1.1', $e->getMessage());
39+
}
40+
try {
41+
$intType->serialize(-1.1);
42+
$this->fail('Expected exception not thrown');
43+
} catch (InvariantViolation $e) {
44+
$this->assertEquals('Int cannot represent non-integer value: -1.1', $e->getMessage());
45+
}
46+
try {
47+
$intType->serialize('-1.1');
48+
$this->fail('Expected exception not thrown');
49+
} catch (InvariantViolation $e) {
50+
$this->assertEquals('Int cannot represent non-integer value: "-1.1"', $e->getMessage());
51+
}
52+
2653
// Maybe a safe PHP int, but bigger than 2^32, so not
2754
// representable as a GraphQL Int
2855
try {
2956
$intType->serialize(9876504321);
3057
$this->fail('Expected exception was not thrown');
31-
} catch (UserError $e) {
58+
} catch (InvariantViolation $e) {
3259
$this->assertEquals('Int cannot represent non 32-bit signed integer value: 9876504321', $e->getMessage());
3360
}
3461

3562
try {
3663
$intType->serialize(-9876504321);
3764
$this->fail('Expected exception was not thrown');
38-
} catch (UserError $e) {
65+
} catch (InvariantViolation $e) {
3966
$this->assertEquals('Int cannot represent non 32-bit signed integer value: -9876504321', $e->getMessage());
4067
}
4168

4269
try {
4370
$intType->serialize(1e100);
4471
$this->fail('Expected exception was not thrown');
45-
} catch (UserError $e) {
72+
} catch (InvariantViolation $e) {
4673
$this->assertEquals('Int cannot represent non 32-bit signed integer value: 1.0E+100', $e->getMessage());
4774
}
4875

4976
try {
5077
$intType->serialize(-1e100);
5178
$this->fail('Expected exception was not thrown');
52-
} catch (UserError $e) {
79+
} catch (InvariantViolation $e) {
5380
$this->assertEquals('Int cannot represent non 32-bit signed integer value: -1.0E+100', $e->getMessage());
5481
}
5582

56-
$this->assertSame(-1, $intType->serialize('-1.1'));
57-
5883
try {
5984
$intType->serialize('one');
6085
$this->fail('Expected exception was not thrown');
61-
} catch (UserError $e) {
86+
} catch (InvariantViolation $e) {
6287
$this->assertEquals('Int cannot represent non 32-bit signed integer value: "one"', $e->getMessage());
6388
}
6489

6590
try {
6691
$intType->serialize('');
6792
$this->fail('Expected exception was not thrown');
68-
} catch (UserError $e) {
93+
} catch (InvariantViolation $e) {
6994
$this->assertEquals('Int cannot represent non 32-bit signed integer value: (empty string)', $e->getMessage());
7095
}
7196

‎tests/Utils/AstFromValueTest.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,17 @@ public function testConvertsBooleanValueToASTs()
4040
public function testConvertsIntValuesToASTs()
4141
{
4242
$this->assertEquals(new IntValueNode(['value' => '123']), AST::astFromValue(123.0, Type::int()));
43-
$this->assertEquals(new IntValueNode(['value' => '123']), AST::astFromValue(123.5, Type::int()));
4443
$this->assertEquals(new IntValueNode(['value' => '10000']), AST::astFromValue(1e4, Type::int()));
44+
$this->assertEquals(new IntValueNode(['value' => '0']), AST::astFromValue(0e4, Type::int()));
45+
46+
// GraphQL spec does not allow coercing non-integer values to Int to avoid
47+
// accidental data loss.
48+
try {
49+
AST::astFromValue(123.5, Type::int());
50+
$this->fail('Expected exception not thrown');
51+
} catch (\Exception $e) {
52+
$this->assertEquals('Int cannot represent non-integer value: 123.5', $e->getMessage());
53+
}
4554

4655
try {
4756
AST::astFromValue(1e40, Type::int()); // Note: js version will produce 1e+40, both values are valid GraphQL floats
@@ -61,6 +70,7 @@ public function testConvertsFloatValuesToIntOrFloatASTs()
6170
$this->assertEquals(new FloatValueNode(['value' => '123.5']), AST::astFromValue(123.5, Type::float()));
6271
$this->assertEquals(new IntValueNode(['value' => '10000']), AST::astFromValue(1e4, Type::float()));
6372
$this->assertEquals(new FloatValueNode(['value' => '1e+40']), AST::astFromValue(1e40, Type::float()));
73+
$this->assertEquals(new IntValueNode(['value' => '0']), AST::astFromValue(0e40, Type::float()));
6474
}
6575

6676
/**

‎tests/Utils/IsValidPHPValueTest.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
namespace GraphQL\Tests\Utils;
3+
4+
5+
use GraphQL\Executor\Values;
6+
use GraphQL\Type\Definition\Type;
7+
8+
class IsValidPHPValueTest extends \PHPUnit_Framework_TestCase
9+
{
10+
public function testValidIntValue()
11+
{
12+
// returns no error for int input
13+
$result = Values::isValidPHPValue('1', Type::int());
14+
$this->expectNoErrors($result);
15+
16+
// returns no error for negative int input:
17+
$result = Values::isValidPHPValue('-1', Type::int());
18+
$this->expectNoErrors($result);
19+
20+
// returns no error for exponent input:
21+
$result = Values::isValidPHPValue('1e3', Type::int());
22+
$this->expectNoErrors($result);
23+
$result = Values::isValidPHPValue('0e3', Type::int());
24+
$this->expectNoErrors($result);
25+
26+
// returns no error for null value:
27+
$result = Values::isValidPHPValue(null, Type::int());
28+
$this->expectNoErrors($result);
29+
30+
// returns a single error for empty value
31+
$result = Values::isValidPHPValue('', Type::int());
32+
$this->expectErrorResult($result, 1);
33+
34+
// returns error for float input as int
35+
$result = Values::isValidPHPValue('1.5', Type::int());
36+
$this->expectErrorResult($result, 1);
37+
38+
// returns a single error for char input
39+
$result = Values::isValidPHPValue('a', Type::int());
40+
$this->expectErrorResult($result, 1);
41+
42+
// returns a single error for char input
43+
$result = Values::isValidPHPValue('meow', Type::int());
44+
$this->expectErrorResult($result, 1);
45+
}
46+
47+
public function testValidFloatValue()
48+
{
49+
// returns no error for int input
50+
$result = Values::isValidPHPValue('1', Type::float());
51+
$this->expectNoErrors($result);
52+
53+
// returns no error for exponent input
54+
$result = Values::isValidPHPValue('1e3', Type::float());
55+
$this->expectNoErrors($result);
56+
$result = Values::isValidPHPValue('0e3', Type::float());
57+
$this->expectNoErrors($result);
58+
59+
// returns no error for float input
60+
$result = Values::isValidPHPValue('1.5', Type::float());
61+
$this->expectNoErrors($result);
62+
63+
// returns no error for null value:
64+
$result = Values::isValidPHPValue(null, Type::float());
65+
$this->expectNoErrors($result);
66+
67+
// returns a single error for empty value
68+
$result = Values::isValidPHPValue('', Type::float());
69+
$this->expectErrorResult($result, 1);
70+
71+
// returns a single error for char input
72+
$result = Values::isValidPHPValue('a', Type::float());
73+
$this->expectErrorResult($result, 1);
74+
75+
// returns a single error for char input
76+
$result = Values::isValidPHPValue('meow', Type::float());
77+
$this->expectErrorResult($result, 1);
78+
}
79+
80+
private function expectNoErrors($result)
81+
{
82+
$this->assertInternalType('array', $result);
83+
$this->assertEquals([], $result);
84+
}
85+
86+
private function expectErrorResult($result, $size) {
87+
$this->assertInternalType('array', $result);
88+
$this->assertEquals($size, count($result));
89+
}
90+
}

0 commit comments

Comments
 (0)