Skip to content

Commit

Permalink
fix(js): use objects as maps safely (#1336)
Browse files Browse the repository at this point in the history
closes #1107 and #1108

 - Use Object.create(null) for maps to remove prototype, which allows use
of in operator, former modelled after use in graphql-js, latter code
style choice to show that object is being used as a map.
 - Create utility for Object.prototype.hasOwnProperty.call
 - Use graphql-js toObjMap, keyMap, and keyValMap helpers to streamline
code.
  • Loading branch information
yaacovCR authored Apr 3, 2020
1 parent 0354e91 commit 1ab52ef
Show file tree
Hide file tree
Showing 40 changed files with 316 additions and 302 deletions.
10 changes: 4 additions & 6 deletions src/delegate/createRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
import { ICreateRequestFromInfo, Request, ICreateRequest } from '../Interfaces';
import { serializeInputValue } from '../utils/index';
import { updateArgument } from '../utils/updateArgument';
import { keyMap } from '../utils/keyMap';

export function getDelegatingOperation(
parentType: GraphQLObjectType,
Expand Down Expand Up @@ -94,8 +95,8 @@ export function createRequest({
argumentNodes = [];
}

const newVariables = {};
const variableDefinitionMap = {};
const newVariables = Object.create(null);
const variableDefinitionMap = Object.create(null);
variableDefinitions.forEach((def) => {
const varName = def.variable.name.value;
variableDefinitionMap[varName] = def;
Expand All @@ -109,10 +110,7 @@ export function createRequest({
);
});

const argumentNodeMap: Record<string, ArgumentNode> = {};
argumentNodes.forEach((argument: ArgumentNode) => {
argumentNodeMap[argument.name.value] = argument;
});
const argumentNodeMap = keyMap(argumentNodes, (arg) => arg.name.value);

updateArgumentsWithDefaults(
sourceParentType,
Expand Down
2 changes: 1 addition & 1 deletion src/delegate/results/handleList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export function handleList(
getNullableType(type.ofType),
listMember,
index,
childErrors[index] != null ? childErrors[index] : [],
index in childErrors ? childErrors[index] : [],
subschema,
context,
info,
Expand Down
2 changes: 1 addition & 1 deletion src/delegate/results/handleNull.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function handleNull(
} else if (errors.some((error) => typeof error.path[1] === 'string')) {
const childErrors = getErrorsByPathSegment(errors);

const result = Object.create(null);
const result = {};
Object.keys(childErrors).forEach((pathSegment) => {
result[pathSegment] = handleNull(
fieldNodes,
Expand Down
2 changes: 1 addition & 1 deletion src/delegate/results/handleObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function getFieldsNotInSubschema(
const fieldsNotInSchema: Array<FieldNode> = [];
Object.keys(subFieldNodes).forEach((responseName) => {
subFieldNodes[responseName].forEach((subFieldNode) => {
if (!fields[subFieldNode.name.value]) {
if (!(subFieldNode.name.value in fields)) {
fieldsNotInSchema.push(subFieldNode);
}
});
Expand Down
28 changes: 16 additions & 12 deletions src/generate/addResolversToSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
forEachDefaultValue,
} from '../utils/index';
import { toConfig } from '../polyfills/index';
import keyValMap from '../utils/keyValMap';

import SchemaError from './SchemaError';
import checkForResolveTypeResolver from './checkForResolveTypeResolver';
Expand Down Expand Up @@ -111,18 +112,21 @@ function addResolversToSchema(
const config = toConfig(type);

const values = type.getValues();
const newValues = {};
values.forEach((value) => {
const newValue = Object.keys(resolverValue).includes(value.name)
? resolverValue[value.name]
: value.name;
newValues[value.name] = {
value: newValue,
deprecationReason: value.deprecationReason,
description: value.description,
astNode: value.astNode,
};
});
const newValues = keyValMap(
values,
(value) => value.name,
(value) => {
const newValue = Object.keys(resolverValue).includes(value.name)
? resolverValue[value.name]
: value.name;
return {
value: newValue,
deprecationReason: value.deprecationReason,
description: value.description,
astNode: value.astNode,
};
},
);

// healSchema called later to update all fields to new type
typeMap[typeName] = new GraphQLEnumType({
Expand Down
6 changes: 3 additions & 3 deletions src/links/createServerHttpLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
import FormData, { AppendOptions } from 'form-data';
import fetch from 'node-fetch';

import { AwaitVariablesLink } from './AwaitVariablesLink';
import { hasOwnProperty } from '../utils/hasOwnProperty';

const hasOwn = Object.prototype.hasOwnProperty;
import { AwaitVariablesLink } from './AwaitVariablesLink';

class FormDataWithStreamSupport extends FormData {
private hasUnknowableLength: boolean;
Expand All @@ -38,7 +38,7 @@ class FormDataWithStreamSupport extends FormData {
!Buffer.isBuffer(value) &&
typeof value !== 'string' &&
!value.path &&
!(value.readable && hasOwn.call(value, 'httpVersion'))
!(value.readable && hasOwnProperty(value, 'httpVersion'))
) {
this.hasUnknowableLength = true;
}
Expand Down
47 changes: 21 additions & 26 deletions src/polyfills/toConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import {
} from 'graphql';

import { graphqlVersion } from '../utils/graphqlVersion';
import { hasOwnProperty } from '../utils/hasOwnProperty';
import keyValMap from '../utils/keyValMap';

export function schemaToConfig(schema: GraphQLSchema): GraphQLSchemaConfig {
if (schema.toConfig != null) {
Expand Down Expand Up @@ -262,17 +264,17 @@ export function enumTypeToConfig(type: GraphQLEnumType): GraphQLEnumTypeConfig {
return type.toConfig();
}

const newValues = {};

type.getValues().forEach((value) => {
newValues[value.name] = {
const newValues = keyValMap(
type.getValues(),
(value) => value.name,
(value) => ({
description: value.description,
value: value.value,
deprecationReason: value.deprecationReason,
extensions: value.extensions,
astNode: value.astNode,
};
});
}),
);

const typeConfig = {
name: type.name,
Expand All @@ -287,8 +289,6 @@ export function enumTypeToConfig(type: GraphQLEnumType): GraphQLEnumTypeConfig {
return typeConfig;
}

const hasOwn = Object.prototype.hasOwnProperty;

export function scalarTypeToConfig(
type: GraphQLScalarType,
): GraphQLScalarTypeConfig<any, any> {
Expand All @@ -300,19 +300,19 @@ export function scalarTypeToConfig(
name: type.name,
description: type.description,
serialize:
graphqlVersion() >= 14 || hasOwn.call(type, 'serialize')
graphqlVersion() >= 14 || hasOwnProperty(type, 'serialize')
? type.serialize
: ((type as unknown) as {
_scalarConfig: GraphQLScalarTypeConfig<any, any>;
})._scalarConfig.serialize,
parseValue:
graphqlVersion() >= 14 || hasOwn.call(type, 'parseValue')
graphqlVersion() >= 14 || hasOwnProperty(type, 'parseValue')
? type.parseValue
: ((type as unknown) as {
_scalarConfig: GraphQLScalarTypeConfig<any, any>;
})._scalarConfig.parseValue,
parseLiteral:
graphqlVersion() >= 14 || hasOwn.call(type, 'parseLiteral')
graphqlVersion() >= 14 || hasOwnProperty(type, 'parseLiteral')
? type.parseLiteral
: ((type as unknown) as {
_scalarConfig: GraphQLScalarTypeConfig<any, any>;
Expand Down Expand Up @@ -349,13 +349,11 @@ export function inputObjectTypeToConfig(
export function inputFieldMapToConfig(
fields: GraphQLInputFieldMap,
): GraphQLInputFieldConfigMap {
const newFields = {};
Object.keys(fields).forEach((fieldName) => {
const field = fields[fieldName];
newFields[fieldName] = toConfig(field);
});

return newFields;
return keyValMap(
Object.keys(fields),
(fieldName) => fieldName,
(fieldName) => toConfig(fields[fieldName]),
);
}

export function inputFieldToConfig(
Expand Down Expand Up @@ -394,14 +392,11 @@ export function directiveToConfig(
export function fieldMapToConfig(
fields: GraphQLFieldMap<any, any>,
): GraphQLFieldConfigMap<any, any> {
const newFields = {};

Object.keys(fields).forEach((fieldName) => {
const field = fields[fieldName];
newFields[fieldName] = toConfig(field);
});

return newFields;
return keyValMap(
Object.keys(fields),
(fieldName) => fieldName,
(fieldName) => toConfig(fields[fieldName]),
);
}

export function fieldToConfig(
Expand Down
2 changes: 1 addition & 1 deletion src/stitch/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function getErrorsByPathSegment(

const pathSegment = error.path[1];

const current = record[pathSegment] != null ? record[pathSegment] : [];
const current = pathSegment in record ? record[pathSegment] : [];
current.push(slicedError(error));
record[pathSegment] = current;
});
Expand Down
45 changes: 26 additions & 19 deletions src/stitch/mergeInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
} from '../utils/index';

import delegateToSchema from '../delegate/delegateToSchema';
import { hasOwnProperty } from '../utils/hasOwnProperty';

type MergeTypeCandidate = {
type: GraphQLNamedType;
Expand Down Expand Up @@ -98,7 +99,7 @@ function createMergedTypes(
mergeTypeCandidates: Array<MergeTypeCandidate>,
) => boolean),
): Record<string, MergedTypeInfo> {
const mergedTypes: Record<string, MergedTypeInfo> = {};
const mergedTypes: Record<string, MergedTypeInfo> = Object.create(null);

Object.keys(typeCandidates).forEach((typeName) => {
if (isObjectType(typeCandidates[typeName][0].type)) {
Expand All @@ -107,7 +108,7 @@ function createMergedTypes(
typeCandidate.subschema != null &&
isSubschemaConfig(typeCandidate.subschema) &&
typeCandidate.subschema.merge != null &&
typeCandidate.subschema.merge[typeName] != null,
hasOwnProperty(typeCandidate.subschema.merge, typeName),
);

if (
Expand Down Expand Up @@ -135,7 +136,7 @@ function createMergedTypes(
) as GraphQLObjectType;
const fieldMap = type.getFields();
Object.keys(fieldMap).forEach((fieldName) => {
if (fields[fieldName] == null) {
if (!(fieldName in fields)) {
fields[fieldName] = [];
}
fields[fieldName].push(subschemaConfig);
Expand Down Expand Up @@ -243,18 +244,20 @@ export function completeMergeInfo(
const field = type[fieldName];
if (field.selectionSet) {
const selectionSet = parseSelectionSet(field.selectionSet);
if (replacementSelectionSets[typeName] == null) {
replacementSelectionSets[typeName] = {};
if (!(typeName in replacementSelectionSets)) {
replacementSelectionSets[typeName] = Object.create(null);
}
if (replacementSelectionSets[typeName][fieldName] == null) {
replacementSelectionSets[typeName][fieldName] = {

const typeReplacementSelectionSets = replacementSelectionSets[typeName];
if (!(fieldName in typeReplacementSelectionSets)) {
typeReplacementSelectionSets[fieldName] = {
kind: Kind.SELECTION_SET,
selections: [],
};
}
replacementSelectionSets[typeName][
typeReplacementSelectionSets[
fieldName
].selections = replacementSelectionSets[typeName][
].selections = typeReplacementSelectionSets[
fieldName
].selections.concat(selectionSet.selections);
}
Expand All @@ -267,26 +270,30 @@ export function completeMergeInfo(
});
});

const mapping = {};
const mapping = Object.create(null);
mergeInfo.fragments.forEach(({ field, fragment }) => {
const parsedFragment = parseFragmentToInlineFragment(fragment);
const actualTypeName = parsedFragment.typeCondition.name.value;
if (mapping[actualTypeName] == null) {
mapping[actualTypeName] = {};
if (!(actualTypeName in mapping)) {
mapping[actualTypeName] = Object.create(null);
}
if (mapping[actualTypeName][field] == null) {
mapping[actualTypeName][field] = [];

const typeMapping = mapping[actualTypeName];
if (!(field in typeMapping)) {
typeMapping[field] = [];
}
mapping[actualTypeName][field].push(parsedFragment);
typeMapping[field].push(parsedFragment);
});

const replacementFragments = Object.create(null);
Object.keys(mapping).forEach((typeName) => {
Object.keys(mapping[typeName]).forEach((field) => {
if (replacementFragments[typeName] == null) {
replacementFragments[typeName] = {};
if (!(typeName in replacementFragments)) {
replacementFragments[typeName] = Object.create(null);
}
replacementFragments[typeName][field] = concatInlineFragments(

const typeReplacementFragments = replacementFragments[typeName];
typeReplacementFragments[field] = concatInlineFragments(
typeName,
mapping[typeName][field],
);
Expand Down Expand Up @@ -321,7 +328,7 @@ function guessSchemaByRootField(
const rootObject = operationToRootType(operation, schema);
if (rootObject != null) {
const fields = rootObject.getFields();
if (fields[fieldName] != null) {
if (fieldName in fields) {
return schema;
}
}
Expand Down
13 changes: 8 additions & 5 deletions src/stitch/mergeSchemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,11 @@ export default function mergeSchemas({
subscriptionTypeName?: string;
}): GraphQLSchema {
const allSchemas: Array<GraphQLSchema> = [];
const typeCandidates: { [name: string]: Array<MergeTypeCandidate> } = {};
const typeMap: { [name: string]: GraphQLNamedType } = {};
const typeCandidates: Record<
string,
Array<MergeTypeCandidate>
> = Object.create(null);
const typeMap: Record<string, GraphQLNamedType> = Object.create(null);
const extensions: Array<DocumentNode> = [];
const directives: Array<GraphQLDirective> = [];

Expand Down Expand Up @@ -228,7 +231,7 @@ export default function mergeSchemas({
(typeof mergeTypes === 'function' &&
mergeTypes(typeName, typeCandidates[typeName])) ||
(Array.isArray(mergeTypes) && mergeTypes.includes(typeName)) ||
mergeInfo.mergedTypes[typeName] != null
typeName in mergeInfo.mergedTypes
) {
typeMap[typeName] = merge(typeName, typeCandidates[typeName]);
} else {
Expand Down Expand Up @@ -292,11 +295,11 @@ export default function mergeSchemas({
}

function addTypeCandidate(
typeCandidates: { [name: string]: Array<MergeTypeCandidate> },
typeCandidates: Record<string, Array<MergeTypeCandidate>>,
name: string,
typeCandidate: MergeTypeCandidate,
) {
if (!typeCandidates[name]) {
if (!(name in typeCandidates)) {
typeCandidates[name] = [];
}
typeCandidates[name].push(typeCandidate);
Expand Down
Loading

0 comments on commit 1ab52ef

Please sign in to comment.