Skip to content

Commit 6857707

Browse files
Scalars: reject array serialization/coercion (#1336)
Working on #1333 I noticed that you can pass arrays as scalars in a query variable, e.g. [SWAPI example](http://graphql.org/swapi-graphql/?query=query%20(%24id%3A%20ID)%20%7B%0A%20%20person(personID%3A%20%24id)%20%7B%0A%20%20%20%20name%0A%20%20%7D%0A%7D&variables=%7B%0A%20%20%22id%22%3A%20%5B1%5D%0A%7D) This PR is based on #925 but fixing same issue for types other than `String` e.g. ```js Number([1]) // 1 ```
1 parent ddb3ffc commit 6857707

File tree

4 files changed

+122
-98
lines changed

4 files changed

+122
-98
lines changed

src/execution/__tests__/variables-test.js

-7
Original file line numberDiff line numberDiff line change
@@ -648,13 +648,6 @@ describe('Execute: Handles inputs', () => {
648648
expect(result.errors[0].originalError).not.to.equal(undefined);
649649
});
650650

651-
it('serializing an array via GraphQLString throws TypeError', () => {
652-
expect(() => GraphQLString.serialize([1, 2, 3])).to.throw(
653-
TypeError,
654-
'String cannot represent an array value: [1,2,3]',
655-
);
656-
});
657-
658651
it('reports error for non-provided variables for non-nullable inputs', () => {
659652
// Note: this test would typically fail validation before encountering
660653
// this execution error, however for queries which previously validated

src/type/__tests__/serialization-test.js

+38-15
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,27 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import { GraphQLInt, GraphQLFloat, GraphQLString, GraphQLBoolean } from '../';
8+
import {
9+
GraphQLInt,
10+
GraphQLID,
11+
GraphQLFloat,
12+
GraphQLString,
13+
GraphQLBoolean,
14+
} from '../';
915

1016
import { describe, it } from 'mocha';
1117
import { expect } from 'chai';
1218

1319
describe('Type System: Scalar coercion', () => {
14-
it('serializes output int', () => {
20+
it('serializes output as Int', () => {
1521
expect(GraphQLInt.serialize(1)).to.equal(1);
1622
expect(GraphQLInt.serialize('123')).to.equal(123);
1723
expect(GraphQLInt.serialize(0)).to.equal(0);
1824
expect(GraphQLInt.serialize(-1)).to.equal(-1);
1925
expect(GraphQLInt.serialize(1e5)).to.equal(100000);
26+
expect(GraphQLInt.serialize(false)).to.equal(0);
27+
expect(GraphQLInt.serialize(true)).to.equal(1);
28+
2029
// The GraphQL specification does not allow serializing non-integer values
2130
// as Int to avoid accidental data loss.
2231
expect(() => GraphQLInt.serialize(0.1)).to.throw(
@@ -49,17 +58,19 @@ describe('Type System: Scalar coercion', () => {
4958
expect(() => GraphQLInt.serialize('one')).to.throw(
5059
'Int cannot represent non 32-bit signed integer value: one',
5160
);
52-
expect(GraphQLInt.serialize(false)).to.equal(0);
53-
expect(GraphQLInt.serialize(true)).to.equal(1);
61+
// Doesn't represent number
5462
expect(() => GraphQLInt.serialize('')).to.throw(
5563
'Int cannot represent non 32-bit signed integer value: (empty string)',
5664
);
5765
expect(() => GraphQLInt.serialize(NaN)).to.throw(
5866
'Int cannot represent non 32-bit signed integer value: NaN',
5967
);
68+
expect(() => GraphQLInt.serialize([5])).to.throw(
69+
'Int cannot represent an array value: [5]',
70+
);
6071
});
6172

62-
it('serializes output float', () => {
73+
it('serializes output as Float', () => {
6374
expect(GraphQLFloat.serialize(1)).to.equal(1.0);
6475
expect(GraphQLFloat.serialize(0)).to.equal(0.0);
6576
expect(GraphQLFloat.serialize('123.5')).to.equal(123.5);
@@ -74,30 +85,42 @@ describe('Type System: Scalar coercion', () => {
7485
expect(() => GraphQLFloat.serialize(NaN)).to.throw(
7586
'Float cannot represent non numeric value: NaN',
7687
);
77-
7888
expect(() => GraphQLFloat.serialize('one')).to.throw(
7989
'Float cannot represent non numeric value: one',
8090
);
81-
8291
expect(() => GraphQLFloat.serialize('')).to.throw(
8392
'Float cannot represent non numeric value: (empty string)',
8493
);
94+
expect(() => GraphQLFloat.serialize([5])).to.throw(
95+
'Float cannot represent an array value: [5]',
96+
);
8597
});
8698

87-
it('serializes output strings', () => {
88-
expect(GraphQLString.serialize('string')).to.equal('string');
89-
expect(GraphQLString.serialize(1)).to.equal('1');
90-
expect(GraphQLString.serialize(-1.1)).to.equal('-1.1');
91-
expect(GraphQLString.serialize(true)).to.equal('true');
92-
expect(GraphQLString.serialize(false)).to.equal('false');
93-
});
99+
for (const scalar of [GraphQLString, GraphQLID]) {
100+
it(`serializes output as ${scalar}`, () => {
101+
expect(scalar.serialize('string')).to.equal('string');
102+
expect(scalar.serialize(1)).to.equal('1');
103+
expect(scalar.serialize(-1.1)).to.equal('-1.1');
104+
expect(scalar.serialize(true)).to.equal('true');
105+
expect(scalar.serialize(false)).to.equal('false');
94106

95-
it('serializes output boolean', () => {
107+
expect(() => scalar.serialize([1])).to.throw(
108+
'String cannot represent an array value: [1]',
109+
);
110+
});
111+
}
112+
113+
it('serializes output as Boolean', () => {
96114
expect(GraphQLBoolean.serialize('string')).to.equal(true);
115+
expect(GraphQLBoolean.serialize('false')).to.equal(true);
97116
expect(GraphQLBoolean.serialize('')).to.equal(false);
98117
expect(GraphQLBoolean.serialize(1)).to.equal(true);
99118
expect(GraphQLBoolean.serialize(0)).to.equal(false);
100119
expect(GraphQLBoolean.serialize(true)).to.equal(true);
101120
expect(GraphQLBoolean.serialize(false)).to.equal(false);
121+
122+
expect(() => GraphQLBoolean.serialize([false])).to.throw(
123+
'Boolean cannot represent an array value: [false]',
124+
);
102125
});
103126
});

src/type/scalars.js

+26-7
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,12 @@ import { Kind } from '../language/kinds';
1919
const MAX_INT = 2147483647;
2020
const MIN_INT = -2147483648;
2121

22-
function coerceInt(value: mixed): ?number {
22+
function coerceInt(value: mixed): number {
23+
if (Array.isArray(value)) {
24+
throw new TypeError(
25+
`Int cannot represent an array value: [${String(value)}]`,
26+
);
27+
}
2328
if (value === '') {
2429
throw new TypeError(
2530
'Int cannot represent non 32-bit signed integer value: (empty string)',
@@ -58,7 +63,12 @@ export const GraphQLInt = new GraphQLScalarType({
5863
},
5964
});
6065

61-
function coerceFloat(value: mixed): ?number {
66+
function coerceFloat(value: mixed): number {
67+
if (Array.isArray(value)) {
68+
throw new TypeError(
69+
`Float cannot represent an array value: [${String(value)}]`,
70+
);
71+
}
6272
if (value === '') {
6373
throw new TypeError(
6474
'Float cannot represent non numeric value: (empty string)',
@@ -88,7 +98,7 @@ export const GraphQLFloat = new GraphQLScalarType({
8898
},
8999
});
90100

91-
function coerceString(value: mixed): ?string {
101+
function coerceString(value: mixed): string {
92102
if (Array.isArray(value)) {
93103
throw new TypeError(
94104
`String cannot represent an array value: ${inspect(value)}`,
@@ -110,11 +120,20 @@ export const GraphQLString = new GraphQLScalarType({
110120
},
111121
});
112122

123+
function coerceBoolean(value: mixed): boolean {
124+
if (Array.isArray(value)) {
125+
throw new TypeError(
126+
`Boolean cannot represent an array value: [${String(value)}]`,
127+
);
128+
}
129+
return Boolean(value);
130+
}
131+
113132
export const GraphQLBoolean = new GraphQLScalarType({
114133
name: 'Boolean',
115134
description: 'The `Boolean` scalar type represents `true` or `false`.',
116-
serialize: Boolean,
117-
parseValue: Boolean,
135+
serialize: coerceBoolean,
136+
parseValue: coerceBoolean,
118137
parseLiteral(ast) {
119138
return ast.kind === Kind.BOOLEAN ? ast.value : undefined;
120139
},
@@ -128,8 +147,8 @@ export const GraphQLID = new GraphQLScalarType({
128147
'response as a String; however, it is not intended to be human-readable. ' +
129148
'When expected as an input type, any string (such as `"4"`) or integer ' +
130149
'(such as `4`) input value will be accepted as an ID.',
131-
serialize: String,
132-
parseValue: String,
150+
serialize: coerceString,
151+
parseValue: coerceString,
133152
parseLiteral(ast) {
134153
return ast.kind === Kind.STRING || ast.kind === Kind.INT
135154
? ast.value

0 commit comments

Comments
 (0)