Skip to content

Commit

Permalink
Validate variables at the gateway level (#3213)
Browse files Browse the repository at this point in the history
Previously, the gateway depended on downstream services to report any
errors with graphql execution. In the case of invalid variables, it's
not necessary to make any fetches or even build a query plan since
we can perform that validation when the request is first received.

Validating at the gateway also guarantees that we surface invalid queries to users,
independent of how the downstream services are implemented and what
kind of validation _they_ perform on incoming requests.
  • Loading branch information
trevor-scheer authored Aug 27, 2019
1 parent a0290c2 commit e151191
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 5 deletions.
38 changes: 38 additions & 0 deletions packages/apollo-gateway/src/__tests__/gateway/executor.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import gql from 'graphql-tag';
import { ApolloGateway } from '../../';

import * as accounts from '../__fixtures__/schemas/accounts';
import * as books from '../__fixtures__/schemas/books';
import * as inventory from '../__fixtures__/schemas/inventory';
import * as product from '../__fixtures__/schemas/product';
import * as reviews from '../__fixtures__/schemas/reviews';

describe('ApolloGateway executor', () => {
it('validates requests prior to execution', async () => {
const gateway = new ApolloGateway({
localServiceList: [accounts, books, inventory, product, reviews],
});

const { executor } = await gateway.load();

const { errors } = await executor({
document: gql`
query InvalidVariables($first: Int!) {
topReviews(first: $first) {
body
}
}
`,
request: {
variables: { first: '3' },
},
queryHash: 'hashed',
context: null,
cache: {} as any,
});

expect(errors![0].message).toMatch(
'Variable "$first" got invalid value "3"; Expected type Int.',
);
});
});
45 changes: 40 additions & 5 deletions packages/apollo-gateway/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
isIntrospectionType,
GraphQLSchema,
GraphQLError,
VariableDefinitionNode,
} from 'graphql';
import { GraphQLSchemaValidationError } from 'apollo-graphql';
import { composeAndValidate, ServiceDefinition } from '@apollo/federation';
Expand All @@ -38,6 +39,7 @@ import { serializeQueryPlan, QueryPlan, OperationContext } from './QueryPlan';
import { GraphQLDataSource } from './datasources/types';
import { RemoteGraphQLDataSource } from './datasources/RemoteGraphQLDataSource';
import { HeadersInit } from 'node-fetch';
import { getVariableValues } from 'graphql/execution/values';

export type ServiceEndpointDefinition = Pick<ServiceDefinition, 'name' | 'url'>;

Expand Down Expand Up @@ -132,6 +134,11 @@ export type UpdateServiceDefinitions = (

type Await<T> = T extends Promise<infer U> ? U : T;

type RequestContext<TContext> = WithRequired<
GraphQLRequestContext<TContext>,
'document' | 'queryHash'
>;

export class ApolloGateway implements GraphQLService {
public schema?: GraphQLSchema;
protected serviceMap: ServiceMap = Object.create(null);
Expand Down Expand Up @@ -439,10 +446,7 @@ export class ApolloGateway implements GraphQLService {
// are unlikely to show up as GraphQLErrors. Do we need to use
// formatApolloErrors or something?
public executor = async <TContext>(
requestContext: WithRequired<
GraphQLRequestContext<TContext>,
'document' | 'operation' | 'queryHash'
>,
requestContext: RequestContext<TContext>,
): Promise<GraphQLExecutionResult> => {
const { request, document, queryHash } = requestContext;
const queryPlanStoreKey = queryHash + (request.operationName || '');
Expand All @@ -451,7 +455,19 @@ export class ApolloGateway implements GraphQLService {
document,
request.operationName,
);
let queryPlan;

// No need to build a query plan if we know the request is invalid beforehand
// In the future, this should be controlled by the requestPipeline
const validationErrors = this.validateIncomingRequest(
requestContext,
operationContext,
);

if (validationErrors.length > 0) {
return { errors: validationErrors };
}

let queryPlan: QueryPlan | undefined;
if (this.queryPlanStore) {
queryPlan = await this.queryPlanStore.get(queryPlanStoreKey);
}
Expand Down Expand Up @@ -510,6 +526,25 @@ export class ApolloGateway implements GraphQLService {
return response;
};

protected validateIncomingRequest<TContext>(
requestContext: RequestContext<TContext>,
operationContext: OperationContext,
) {
// casting out of `readonly`
const variableDefinitions = operationContext.operation
.variableDefinitions as VariableDefinitionNode[] | undefined;

if (!variableDefinitions) return [];

const { errors } = getVariableValues(
operationContext.schema,
variableDefinitions,
requestContext.request.variables!,
);

return errors || [];
}

private initializeQueryPlanStore(): void {
this.queryPlanStore = new InMemoryLRUCache<QueryPlan>({
// Create ~about~ a 30MiB InMemoryLRUCache. This is less than precise
Expand Down

0 comments on commit e151191

Please sign in to comment.