Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added stricter scalar type checks #320

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,495 changes: 576 additions & 919 deletions package-lock.json

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions packages/openapi-to-graphql/lib/schema_builder.d.ts
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@
*/
import { PreprocessingData } from './types/preprocessing_data';
import { Operation, DataDefinition } from './types/operation';
import { ParameterObject } from './types/oas3';
import { SchemaObject, ParameterObject } from './types/oas3';
import { Args, GraphQLType } from './types/graphql';
declare type GetArgsParams = {
requestPayloadDef?: DataDefinition;
@@ -13,6 +13,7 @@ declare type GetArgsParams = {
};
declare type CreateOrReuseComplexTypeParams = {
def: DataDefinition;
schema?: SchemaObject;
operation?: Operation;
iteration?: number;
isInputObjectType?: boolean;
@@ -21,7 +22,7 @@ declare type CreateOrReuseComplexTypeParams = {
/**
* Creates and returns a GraphQL type for the given JSON schema.
*/
export declare function getGraphQLType({ def, operation, data, iteration, isInputObjectType }: CreateOrReuseComplexTypeParams): GraphQLType;
export declare function getGraphQLType({ def, schema, operation, data, iteration, isInputObjectType }: CreateOrReuseComplexTypeParams): GraphQLType;
/**
* Creates the arguments for resolving a field
*/
130 changes: 123 additions & 7 deletions packages/openapi-to-graphql/lib/schema_builder.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/openapi-to-graphql/lib/schema_builder.js.map

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions packages/openapi-to-graphql/lib/types/oas3.d.ts
Original file line number Diff line number Diff line change
@@ -8,6 +8,11 @@ declare type ExternalDocumentationObject = {
export declare type SchemaObject = {
$ref?: string;
title?: string;
minimum?: number;
maximum?: number;
maxLength?: number;
minLength?: number;
pattern?: string;
type?: 'string' | 'number' | 'object' | 'array' | 'boolean' | 'integer';
format?: string;
nullable?: boolean;
24 changes: 24 additions & 0 deletions packages/openapi-to-graphql/lib/utils.d.ts
Original file line number Diff line number Diff line change
@@ -33,6 +33,30 @@ export declare const mitigations: {
LIMIT_ARGUMENT_NAME_COLLISION: string;
OAUTH_SECURITY_SCHEME: string;
};
/**
* verify that a variable contains a safe int (2^31)
*/
export declare function isSafeInteger(n: unknown): n is number;
/**
* verify that a variable contains a safe long (2^53)
*/
export declare function isSafeLong(n: unknown): n is number;
/**
* verify that a vriable contains a valid UUID string
*/
export declare function isUUID(s: any): boolean;
/**
* verify
*/
export declare function isURL(s: any): boolean;
/**
* verify that a vriable contains a safe date/date-time string
*/
export declare function isSafeDate(n: string): boolean;
/**
* get the correct type of a variable
*/
export declare function strictTypeOf(value: any, type: any): boolean;
/**
* Utilities that are specific to OpenAPI-to-GraphQL
*/
106 changes: 106 additions & 0 deletions packages/openapi-to-graphql/lib/utils.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/openapi-to-graphql/lib/utils.js.map
2,967 changes: 1,529 additions & 1,438 deletions packages/openapi-to-graphql/package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions packages/openapi-to-graphql/package.json
Original file line number Diff line number Diff line change
@@ -69,6 +69,7 @@
"debug": "^4.1.0",
"deep-equal": "^2.0.1",
"form-urlencoded": "^4.1.1",
"graphql-scalar": "0.0.11",
"graphql-subscriptions": "^1.1.0",
"graphql-type-json": "^0.2.1",
"jsonpath-plus": "^3.0.0",
198 changes: 193 additions & 5 deletions packages/openapi-to-graphql/src/schema_builder.ts
Original file line number Diff line number Diff line change
@@ -19,6 +19,17 @@ import {
} from './types/oas3'
import { Args, GraphQLType, ResolveFunction } from './types/graphql'
import {
createStringScalar,
createIntScalar,
createFloatScalar,
ScalarSanitizeFunction,
ScalarValidateFunction,
ScalarCoerceFunction,
ScalarParseFunction
} from 'graphql-scalar'

import {
GraphQLError,
GraphQLScalarType,
GraphQLObjectType,
GraphQLString,
@@ -41,7 +52,16 @@ import * as Oas3Tools from './oas_3_tools'
import { getResolver } from './resolver_builder'
import { createDataDef } from './preprocessor'
import debug from 'debug'
import { handleWarning, sortObject } from './utils'
import {
handleWarning,
sortObject,
isSafeInteger,
isSafeLong,
strictTypeOf,
isSafeDate,
isUUID,
isURL
} from './utils'

type GetArgsParams = {
requestPayloadDef?: DataDefinition
@@ -52,6 +72,7 @@ type GetArgsParams = {

type CreateOrReuseComplexTypeParams = {
def: DataDefinition
schema?: SchemaObject
operation?: Operation
iteration?: number // Count of recursions used to create type
isInputObjectType?: boolean // Does not require isInputObjectType because unions must be composed of objects
@@ -60,6 +81,8 @@ type CreateOrReuseComplexTypeParams = {

type CreateOrReuseSimpleTypeParams = {
def: DataDefinition
schema?: SchemaObject
isInputObjectType?: boolean // Does not require isInputObjectType because unions must be composed of objects
data: PreprocessingData
}

@@ -79,13 +102,43 @@ type LinkOpRefToOpIdParams = {
data: PreprocessingData
}

interface StrictScalarConfig {
name: string
description?: string
maximum?: number
minimum?: number
maxLength?: number
minLength?: number
nonEmpty?: boolean
pattern?: RegExp | string
singleline?: string
trim?: boolean
}

interface StrictScalarNumberConfig extends StrictScalarConfig {
serialize?: ScalarSanitizeFunction<number>
parse?: ScalarParseFunction<number, number>
coerce?: ScalarCoerceFunction<number>
sanitize?: ScalarSanitizeFunction<number>
validate?: ScalarValidateFunction<number>
}

interface StrictScalarStringConfig extends StrictScalarConfig {
serialize?: ScalarSanitizeFunction<string>
parse?: ScalarParseFunction<string, string>
coerce?: ScalarCoerceFunction<string>
sanitize?: ScalarSanitizeFunction<string>
validate?: ScalarValidateFunction<string>
}

const translationLog = debug('translation')

/**
* Creates and returns a GraphQL type for the given JSON schema.
*/
export function getGraphQLType({
def,
schema,
operation,
data,
iteration = 0,
@@ -125,6 +178,7 @@ export function getGraphQLType({
return createOrReuseList({
def,
operation,
schema,
data,
iteration,
isInputObjectType
@@ -141,6 +195,8 @@ export function getGraphQLType({
default:
return getScalarType({
def,
schema,
isInputObjectType,
data
})
}
@@ -422,6 +478,7 @@ function checkAmbiguousMemberTypes(
function createOrReuseList({
def,
operation,
schema,
iteration,
isInputObjectType,
data
@@ -461,6 +518,7 @@ function createOrReuseList({
const itemsType = getGraphQLType({
def: itemDef,
data,
schema,
operation,
iteration: iteration + 1,
isInputObjectType
@@ -523,20 +581,139 @@ function createOrReuseEnum({
*/
function getScalarType({
def,
schema,
isInputObjectType,
data
}: CreateOrReuseSimpleTypeParams): GraphQLScalarType {
const options: StrictScalarNumberConfig | StrictScalarStringConfig = {
name: ''
}

if (isInputObjectType && schema) {
const type = schema.type
const title = schema.title || ''

options.name =
title.split(' ').join('') ||
'StrictScalarType' +
(Math.random() * Date.now()).toString(16).replace('.', '')

if (type === 'string') {
options.trim = true
if ('nullable' in schema) options.nonEmpty = !schema.nullable
}

switch (true) {
case typeof schema.minimum === 'number':
case typeof schema.minLength === 'number':
if (type === 'string') {
options.minLength = schema.minLength
}

if (type === 'number' || type === 'integer') {
options.minimum = schema.minimum
}
break
case typeof schema.maximum === 'number':
case typeof schema.maxLength === 'number':
if (type === 'string') {
options.maxLength = schema.maxLength
}

if (type === 'number' || type === 'integer') {
options.maximum = schema.maximum
}
break
case typeof schema.pattern === 'string':
const $qualifier = schema.pattern.match(/\/(.)$/) || ['', '']
const $pattern = schema.pattern
.replace(/^\//, '')
.replace(/\/(.)?$/, '')

if (type === 'string') {
options.pattern = new RegExp($pattern, $qualifier[1])
}
break
case typeof schema.description === 'string':
options.description = schema.description.replace(/\s/g, '').trim()
break
case type !== 'object' && type !== 'array' && type === 'boolean':
case typeof schema.format === 'string':
case typeof schema.enum !== 'undefined':
const $format = schema.format || '-'
const $enum = schema.enum || []

options.parse = (data: any) => {
if (type === 'string') {
return String(data) as string
}

return data
}

options.coerce = (data: any) => {
if (type === 'number' || $format === 'float') {
if (!isFinite(data)) {
throw new GraphQLError('Float cannot represent non numeric value')
}
}

if (type === 'string') {
if (typeof data !== 'string') {
throw new GraphQLError(
'String cannot represent a non string value'
)
}
}

return data
}

options.sanitize = (data: any) => {
return type === 'integer' || $format.startsWith('int')
? parseInt(data, 10)
: type === 'number' || $format === 'float'
? parseFloat(data)
: $format === 'date' || $format === 'date-time'
? isSafeDate(data) && data
: data
}

options.validate = (data: any) =>
$format === 'int64'
? isSafeLong(data)
: $format === 'int32'
? isSafeInteger(data)
: $format === 'uuid'
? isUUID(data)
: $format === 'url'
? isURL(data)
: $enum.includes(String(data)) || strictTypeOf(data, type)
break
}
}

switch (def.targetGraphQLType) {
case 'id':
def.graphQLType = GraphQLID
break
case 'string':
def.graphQLType = GraphQLString
def.graphQLType =
isInputObjectType && schema
? createStringScalar(options as StrictScalarStringConfig)
: GraphQLString
break
case 'integer':
def.graphQLType = GraphQLInt
def.graphQLType =
isInputObjectType && schema
? createIntScalar(options as StrictScalarNumberConfig)
: GraphQLInt
break
case 'number':
def.graphQLType = GraphQLFloat
def.graphQLType =
isInputObjectType && schema
? createFloatScalar(options as StrictScalarNumberConfig)
: GraphQLFloat
break
case 'boolean':
def.graphQLType = GraphQLBoolean
@@ -577,6 +754,7 @@ function createFields({
const objectType = getGraphQLType({
def: fieldTypeDefinition,
operation,
schema: fieldSchema,
data,
iteration: iteration + 1,
isInputObjectType
@@ -1113,6 +1291,7 @@ export function getArgs({
const type = getGraphQLType({
def: paramDef,
operation,
schema,
data,
iteration: 0,
isInputObjectType: true
@@ -1188,6 +1367,7 @@ export function getArgs({
const reqObjectType = getGraphQLType({
def: requestPayloadDef,
data,
schema: requestPayloadDef.schema,
operation,
isInputObjectType: true // Request payloads will always be an input object type
})
@@ -1204,7 +1384,15 @@ export function getArgs({
: false

args[saneName] = {
type: reqRequired ? new GraphQLNonNull(reqObjectType) : reqObjectType,
type: reqRequired
? new GraphQLNonNull(reqObjectType)
: typeof (requestPayloadDef.schema as SchemaObject).default !==
'undefined'
? {
type: reqObjectType,
defaultValue: (requestPayloadDef.schema as SchemaObject).default
}
: reqObjectType,
// TODO: addendum to the description explaining this is the request body
description: requestPayloadDef.schema.description
}
5 changes: 5 additions & 0 deletions packages/openapi-to-graphql/src/types/oas3.ts
Original file line number Diff line number Diff line change
@@ -17,6 +17,11 @@ type ExternalDocumentationObject = {
export type SchemaObject = {
$ref?: string
title?: string
minimum?: number
maximum?: number
maxLength?: number
minLength?: number
pattern?: string
type?: 'string' | 'number' | 'object' | 'array' | 'boolean' | 'integer'
format?: string
nullable?: boolean
126 changes: 126 additions & 0 deletions packages/openapi-to-graphql/src/utils.ts
Original file line number Diff line number Diff line change
@@ -51,6 +51,132 @@ export const mitigations = {
OAUTH_SECURITY_SCHEME: `Ignore security scheme`
}

const MAX_INT = 2147483647
const MIN_INT = -2147483648

const MAX_LONG = 9007199254740991
const MIN_LONG = -9007199254740992

/**
* verify that a variable contains a safe int (2^31)
*/
export function isSafeInteger(n: unknown): n is number {
return (
typeof n === 'number' &&
isFinite(n) &&
Math.floor(n) === n &&
n <= MAX_INT &&
n >= MIN_INT
)
}

/**
* verify that a variable contains a safe long (2^53)
*/

export function isSafeLong(n: unknown): n is number {
return typeof n === 'number' && isFinite(n) && n <= MAX_LONG && n >= MIN_LONG
}

/**
* verify that a vriable contains a valid UUID string
*/

export function isUUID(s: any): boolean {
const uuidRegExp = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
return uuidRegExp.test(s)
}

/**
* verify
*/

export function isURL(s: any): boolean {
let res = null
try {
res = s.match(
/(http(s)?:\/\/.)?(www\.)?[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z0-9]{2,6}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)/g
)
} catch (e) {
res = null
}
return res !== null
}

/**
* verify that a vriable contains a safe date/date-time string
*/

export function isSafeDate(n: string): boolean {
const parsed = Date.parse(n)
return (
typeof parsed === 'number' &&
parsed !== NaN &&
parsed > 0 &&
String(parsed).length === 13
)
}

/**
* check if a literal is falsy or not
*/
const isLiteralFalsey = (variable): boolean => {
return variable === '' || variable === false || variable === 0
}

/**
* provide the name of primitive and/or reference types
*/
const checkTypeName = (target, type): boolean => {
let typeName = ''

if (isLiteralFalsey(target)) {
typeName = typeof target
} else {
typeName = '' + (target && target.constructor.name)
}
return !!(typeName.toLowerCase().indexOf(type) + 1)
}

/**
* get the correct type of a variable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isocroft Minor: Can you capitalize the first word of comments to be in line with the rest of the codebase?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ErikWittern I have done this

*/
export function strictTypeOf(value, type): boolean {
let result = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition, for other check functions/helper functions, it would be nice to include a reference so we can easily learn more and also verify its completeness.

Copy link
Author

@isocroft isocroft May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was created by me (I own the copyright - 😄😀) in 2015. The only references I have are the many codebases I have used it with resounding success. The function has also been tested in multiple instances and ways. Over the years, it has been re-written and updated for performance/correctness. It basically uses the variables' constructor name to determine what that type the variable is.

Below are example of its implementation:

const isURL = (variable) => strictTypeOf(variable, 'url')
const isBlob = (variable) => strictTypeOf(variable, 'blob')
const isSet = (variable) => strictTypeOf(variable, 'set')
const isMap = (variable) => strictTypeOf(variable, 'map')
const isBuffer = (variable) => strictTypeOf(variable, 'buffer')
const isEmitter = (variable) => strictTypeOf(variable, 'eventemitter')

Copy link
Author

@isocroft isocroft May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I modified it to not inspect the prototype chain as such checks are not required for this project ( or are they ? could they be ? ). Here is the original version of the strictTypeOf implementation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isocroft I think it would be helpful to improve the documentation of this function. Both arguments are typed as any, but isn't at least the second supposed to be a string? The comment for the function states get the correct type of a variable. Could you elaborate on the context? It seems we are comparing a runtime value with what is defined in a JSON schema definition here, is that correct? I think it would also help to add some inline comments within this function - at least I am having issues following what it does.

Copy link
Author

@isocroft isocroft May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ErikWittern the version of the strictTypeOf function pasted above (see GitHub gist link) is not the same version used in the repo and in this PR. As it does a lot of things that is not needed for this project (e.g. looking up the prototype chain). Below is the slimmed-down version being used in the repo and in this PR - I have also added comments:

/**
 * check if a literal is falsy or not
 */
const isLiteralFalsey = (variable: unknown): boolean => {
  return variable === '' || variable === false || variable === 0
}

/**
 * check if a variable contains a reference type (not a literal)
 */

const isPrimitive = (arg: unknown): boolean => {
   return typeof arg === 'object' || Boolean(arg) && typeof arg.apply === 'function'
}

/**
 * provide the name of primitive and/or reference types
 */
const checkTypeName = (target: unknown, type: string): boolean => {
  let typeName = ''
  
  // we need to separate checks for literal types and 
  // primitive types so we can speed up performance and
  // keep things simple
  if (isLiteralFalsey(target) || !isPrimitive(target)) {
    // literal 
    typeName = typeof target
  } else {
    // primitive/reference
    typeName = (Object.prototype.toString.call(target)).replace(/^\[object (.+)\]$/, '$1') 
  }

  return Boolean(typeName.toLowerCase().indexOf(type) + 1)
}

/**
 * get the correct type of a variable
 */
export function strictTypeOf(value: unknown, type: string): boolean {
  // swagger/OpenAPI 'integer' type is converted
  // a JavaScript 'number' type for compatibility
  if (type === 'integer') {
    type = 'number'
  }

  type = type || ''
  return checkTypeName(value, type)
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

examples:

// swagger/OPenAPI types: 'integer', 'string', 'array', 'object'
strictTypeOf(variable, 'integer')

Copy link
Author

@isocroft isocroft Feb 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ErikWittern No, that's not correct. The function isn't comparing a runtime value with what is defined in a JSON schema definition. It is checking that the data type of the variable is as intended/expected. I have added comments and references to this function for more clarity.


if (type === 'integer') {
type = 'number'
}

type = type || []

if (typeof type === 'object') {
if (typeof type.length !== 'number') {
return result
}

let bitPiece = 0

type = [].slice.call(type)

type.forEach(_type => {
if (typeof _type === 'function') {
_type = (_type.name || _type.displayName).toLowerCase()
}
bitPiece |= Number(checkTypeName(value, _type))
})

result = Boolean(bitPiece)
} else {
if (typeof type === 'function') {
type = (type.name || type.displayName).toLowerCase()
}

result = checkTypeName(value, type)
}
return result
}

/**
* Utilities that are specific to OpenAPI-to-GraphQL
*/