Skip to content

Commit

Permalink
RFC: Assert subscription field is not introspection. (#2861)
Browse files Browse the repository at this point in the history
  • Loading branch information
benjie committed Jun 3, 2021
1 parent fd3d8c9 commit d5eac89
Show file tree
Hide file tree
Showing 3 changed files with 289 additions and 14 deletions.
189 changes: 188 additions & 1 deletion src/validation/__tests__/SingleFieldSubscriptionsRule-test.ts
Expand Up @@ -2,7 +2,11 @@ import { describe, it } from 'mocha';

import { SingleFieldSubscriptionsRule } from '../rules/SingleFieldSubscriptionsRule';

import { expectValidationErrors } from './harness';
import {
expectValidationErrors,
expectValidationErrorsWithSchema,
emptySchema,
} from './harness';

function expectErrors(queryStr: string) {
return expectValidationErrors(SingleFieldSubscriptionsRule, queryStr);
Expand All @@ -21,6 +25,41 @@ describe('Validate: Subscriptions with single field', () => {
`);
});

it('valid subscription with fragment', () => {
// From https://spec.graphql.org/draft/#example-13061
expectValid(`
subscription sub {
...newMessageFields
}
fragment newMessageFields on SubscriptionRoot {
newMessage {
body
sender
}
}
`);
});

it('valid subscription with fragment and field', () => {
// From https://spec.graphql.org/draft/#example-13061
expectValid(`
subscription sub {
newMessage {
body
}
...newMessageFields
}
fragment newMessageFields on SubscriptionRoot {
newMessage {
body
sender
}
}
`);
});

it('fails with more than one root field', () => {
expectErrors(`
subscription ImportantEmails {
Expand Down Expand Up @@ -48,6 +87,34 @@ describe('Validate: Subscriptions with single field', () => {
'Subscription "ImportantEmails" must select only one top level field.',
locations: [{ line: 4, column: 9 }],
},
{
message:
'Subscription "ImportantEmails" must not select an introspection top level field.',
locations: [{ line: 4, column: 9 }],
},
]);
});

it('fails with more than one root field including aliased introspection via fragment', () => {
expectErrors(`
subscription ImportantEmails {
importantEmails
...Introspection
}
fragment Introspection on SubscriptionRoot {
typename: __typename
}
`).to.deep.equal([
{
message:
'Subscription "ImportantEmails" must select only one top level field.',
locations: [{ line: 7, column: 9 }],
},
{
message:
'Subscription "ImportantEmails" must not select an introspection top level field.',
locations: [{ line: 7, column: 9 }],
},
]);
});

Expand All @@ -70,6 +137,86 @@ describe('Validate: Subscriptions with single field', () => {
]);
});

it('fails with many more than one root field via fragments', () => {
expectErrors(`
subscription ImportantEmails {
importantEmails
... {
more: moreImportantEmails
}
...NotImportantEmails
}
fragment NotImportantEmails on SubscriptionRoot {
notImportantEmails
deleted: deletedEmails
...SpamEmails
}
fragment SpamEmails on SubscriptionRoot {
spamEmails
}
`).to.deep.equal([
{
message:
'Subscription "ImportantEmails" must select only one top level field.',
locations: [
{ line: 5, column: 11 },
{ line: 10, column: 9 },
{ line: 11, column: 9 },
{ line: 15, column: 9 },
],
},
]);
});

it('does not infinite loop on recursive fragments', () => {
expectErrors(`
subscription NoInfiniteLoop {
...A
}
fragment A on SubscriptionRoot {
...A
}
`).to.deep.equal([]);
});

it('fails with many more than one root field via fragments (anonymous)', () => {
expectErrors(`
subscription {
importantEmails
... {
more: moreImportantEmails
...NotImportantEmails
}
...NotImportantEmails
}
fragment NotImportantEmails on SubscriptionRoot {
notImportantEmails
deleted: deletedEmails
... {
... {
archivedEmails
}
}
...SpamEmails
}
fragment SpamEmails on SubscriptionRoot {
spamEmails
...NonExistentFragment
}
`).to.deep.equal([
{
message: 'Anonymous Subscription must select only one top level field.',
locations: [
{ line: 5, column: 11 },
{ line: 11, column: 9 },
{ line: 12, column: 9 },
{ line: 15, column: 13 },
{ line: 21, column: 9 },
],
},
]);
});

it('fails with more than one root field in anonymous subscriptions', () => {
expectErrors(`
subscription {
Expand All @@ -83,4 +230,44 @@ describe('Validate: Subscriptions with single field', () => {
},
]);
});

it('fails with introspection field', () => {
expectErrors(`
subscription ImportantEmails {
__typename
}
`).to.deep.equal([
{
message:
'Subscription "ImportantEmails" must not select an introspection top level field.',
locations: [{ line: 3, column: 9 }],
},
]);
});

it('fails with introspection field in anonymous subscription', () => {
expectErrors(`
subscription {
__typename
}
`).to.deep.equal([
{
message:
'Anonymous Subscription must not select an introspection top level field.',
locations: [{ line: 3, column: 9 }],
},
]);
});

it('skips if not subscription type', () => {
expectValidationErrorsWithSchema(
emptySchema,
SingleFieldSubscriptionsRule,
`
subscription {
__typename
}
`,
).to.deep.equal([]);
});
});
25 changes: 25 additions & 0 deletions src/validation/__tests__/harness.ts
Expand Up @@ -130,8 +130,23 @@ export const testSchema: GraphQLSchema = buildSchema(`
complicatedArgs: ComplicatedArgs
}
type Message {
body: String
sender: String
}
type SubscriptionRoot {
importantEmails: [String]
notImportantEmails: [String]
moreImportantEmails: [String]
spamEmails: [String]
deletedEmails: [String]
newMessage: Message
}
schema {
query: QueryRoot
subscription: SubscriptionRoot
}
directive @onQuery on QUERY
Expand All @@ -144,6 +159,16 @@ export const testSchema: GraphQLSchema = buildSchema(`
directive @onVariableDefinition on VARIABLE_DEFINITION
`);

export const emptySchema: GraphQLSchema = buildSchema(`
type QueryRoot {
empty: Boolean
}
schema {
query: QueryRoot
}
`);

export function expectValidationErrorsWithSchema(
schema: GraphQLSchema,
rule: ValidationRule,
Expand Down
89 changes: 76 additions & 13 deletions src/validation/rules/SingleFieldSubscriptionsRule.ts
@@ -1,30 +1,93 @@
import type { ObjMap } from '../../jsutils/ObjMap';
import { GraphQLError } from '../../error/GraphQLError';

import type { ASTVisitor } from '../../language/visitor';
import type { OperationDefinitionNode } from '../../language/ast';
import type {
OperationDefinitionNode,
FragmentDefinitionNode,
} from '../../language/ast';
import { Kind } from '../../language/kinds';

import type { ASTValidationContext } from '../ValidationContext';
import type { ValidationContext } from '../ValidationContext';
import type { ExecutionContext } from '../../execution/execute';
import {
collectFields,
defaultFieldResolver,
defaultTypeResolver,
} from '../../execution/execute';

/**
* Subscriptions must only include one field.
* Subscriptions must only include a non-introspection field.
*
* A GraphQL subscription is valid only if it contains a single root field.
* A GraphQL subscription is valid only if it contains a single root field and
* that root field is not an introspection field.
*/
export function SingleFieldSubscriptionsRule(
context: ASTValidationContext,
context: ValidationContext,
): ASTVisitor {
return {
OperationDefinition(node: OperationDefinitionNode) {
if (node.operation === 'subscription') {
if (node.selectionSet.selections.length !== 1) {
context.reportError(
new GraphQLError(
node.name
? `Subscription "${node.name.value}" must select only one top level field.`
: 'Anonymous Subscription must select only one top level field.',
node.selectionSet.selections.slice(1),
),
const schema = context.getSchema();
const subscriptionType = schema.getSubscriptionType();
if (subscriptionType) {
const operationName = node.name ? node.name.value : null;
const variableValues: {
[variable: string]: any;
} = Object.create(null);
const document = context.getDocument();
const fragments: ObjMap<FragmentDefinitionNode> = Object.create(null);
for (const definition of document.definitions) {
if (definition.kind === Kind.FRAGMENT_DEFINITION) {
fragments[definition.name.value] = definition;
}
}
// FIXME: refactor out `collectFields` into utility function that doesn't need fake context.
const fakeExecutionContext: ExecutionContext = {
schema,
fragments,
rootValue: undefined,
contextValue: undefined,
operation: node,
variableValues,
fieldResolver: defaultFieldResolver,
typeResolver: defaultTypeResolver,
errors: [],
};
const fields = collectFields(
fakeExecutionContext,
subscriptionType,
node.selectionSet,
new Map(),
new Set(),
);
if (fields.size > 1) {
const fieldSelectionLists = [...fields.values()];
const extraFieldSelectionLists = fieldSelectionLists.slice(1);
const extraFieldSelections = extraFieldSelectionLists.flat();
context.reportError(
new GraphQLError(
operationName != null
? `Subscription "${operationName}" must select only one top level field.`
: 'Anonymous Subscription must select only one top level field.',
extraFieldSelections,
),
);
}
for (const fieldNodes of fields.values()) {
const field = fieldNodes[0];
const fieldName = field.name.value;
if (fieldName[0] === '_' && fieldName[1] === '_') {
context.reportError(
new GraphQLError(
operationName != null
? `Subscription "${operationName}" must not select an introspection top level field.`
: 'Anonymous Subscription must not select an introspection top level field.',
fieldNodes,
),
);
}
}
}
}
},
Expand Down

0 comments on commit d5eac89

Please sign in to comment.