From 21d47c9a5023de41d6a61695394cea2fbf850ae7 Mon Sep 17 00:00:00 2001 From: Marshall Cottrell Date: Thu, 17 Jan 2019 09:23:57 -0600 Subject: [PATCH] fix(server): throw when encountering `undefined` positional arguments (#958) Fixes #951 --- .../__tests__/postgraphile-test.js | 26 ++++++- src/postgraphile/postgraphile.ts | 67 ++++++++++++------- 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/src/postgraphile/__tests__/postgraphile-test.js b/src/postgraphile/__tests__/postgraphile-test.js index e124468d73..b5446d7eff 100644 --- a/src/postgraphile/__tests__/postgraphile-test.js +++ b/src/postgraphile/__tests__/postgraphile-test.js @@ -20,7 +20,7 @@ test('will use a connected client from the pool, the schemas, and options to cre createPostGraphileHttpRequestHandler.mockClear(); const pgPool = new Pool(); const schemas = [Symbol('schemas')]; - const options = Symbol('options'); + const options = { $options: Symbol('options') }; await postgraphile(pgPool, schemas, options); expect(createPostGraphileSchema.mock.calls).toEqual([[pgPool, schemas, options]]); }); @@ -29,8 +29,7 @@ test('will use a connected client from the pool, the default schema, and options createPostGraphileSchema.mockClear(); createPostGraphileHttpRequestHandler.mockClear(); const pgPool = new Pool(); - const options = Symbol('options'); - const pgClient = { release: jest.fn() }; + const options = { $options: Symbol('options') }; await postgraphile(pgPool, options); expect(createPostGraphileSchema.mock.calls).toEqual([[pgPool, ['public'], options]]); }); @@ -83,3 +82,24 @@ test('will not error if jwtSecret is provided without jwtPgTypeIdentifier', asyn const pgPool = new Pool(); expect(() => postgraphile(pgPool, [], { jwtSecret: 'test' })).not.toThrow(); }); + +test('will throw on undefined positional arguments', async () => { + const pgPool = new Pool(); + const options = { $options: Symbol('options') }; + + createPostGraphileSchema.mockClear(); + expect(() => postgraphile(pgPool, null, options)).not.toThrow(); + expect(() => postgraphile(pgPool, options)).not.toThrow(); + expect(createPostGraphileSchema.mock.calls).toEqual([ + [pgPool, ['public'], options], + [pgPool, ['public'], options], + ]); + + expect(() => postgraphile(null)).not.toThrow(); + expect(() => postgraphile(pgPool, null)).not.toThrow(); + expect(() => postgraphile(null, 'public')).not.toThrow(); + + expect(() => postgraphile(undefined)).toThrow(); + expect(() => postgraphile(pgPool, undefined)).toThrow(); + expect(() => postgraphile(undefined, 'public')).toThrow(); +}); diff --git a/src/postgraphile/postgraphile.ts b/src/postgraphile/postgraphile.ts index 065ded1a7a..e2b1ab9df9 100644 --- a/src/postgraphile/postgraphile.ts +++ b/src/postgraphile/postgraphile.ts @@ -114,41 +114,40 @@ export default function postgraphile( // must process them with `pluginHook` before we can rely on them. let incomingOptions: PostGraphileOptions; - // If the second argument is undefined, use defaults for both `schema` and - // `incomingOptions`. - if (typeof schemaOrOptions === 'undefined') { - schema = 'public'; - incomingOptions = {}; - } // If the second argument is a string or array, it is the schemas so set the // `schema` value and try to use the third argument (or a default) for // `incomingOptions`. - else if (typeof schemaOrOptions === 'string' || Array.isArray(schemaOrOptions)) { + if (typeof schemaOrOptions === 'string' || Array.isArray(schemaOrOptions)) { schema = schemaOrOptions; incomingOptions = maybeOptions || {}; } - // Otherwise the second argument is the incomingOptions so set `schema` to the - // default and `incomingOptions` to the second argument. + // If the second argument is null or an object then use default `schema` + // and set incomingOptions to second or third argument (or default). + else if (typeof schemaOrOptions === 'object') { + schema = 'public'; + incomingOptions = schemaOrOptions || maybeOptions || {}; + } + // Otherwise if the second argument is present it's invalid: throw an error. + else if (arguments.length > 1) { + throw new Error( + 'The second argument to postgraphile was invalid... did you mean to set a schema?', + ); + } + // No schema or options specified, use defaults. else { schema = 'public'; - incomingOptions = schemaOrOptions; + incomingOptions = {}; + } + + if (typeof poolOrConfig === 'undefined' && arguments.length >= 1) { + throw new Error( + 'The first argument to postgraphile was `undefined`... did you mean to set pool options?', + ); } // Do some things with `poolOrConfig` so that in the end, we actually get a // Postgres pool. - const pgPool: Pool = - // If it is already a `Pool`, just use it. - poolOrConfig instanceof Pool || quacksLikePgPool(poolOrConfig) - ? (poolOrConfig as Pool) - : new Pool( - typeof poolOrConfig === 'string' - ? // Otherwise if it is a string, let us parse it to get a config to - // create a `Pool`. - parsePgConnectionString(poolOrConfig) - : // Finally, it must just be a config itself. If it is undefined, we - // will just use an empty config and let the defaults take over. - poolOrConfig || {}, - ); + const pgPool = toPgPool(poolOrConfig); pgPool.on('error', err => { /* @@ -202,7 +201,27 @@ function constructorName(obj: mixed): string | null { } // tslint:disable-next-line no-any -function quacksLikePgPool(pgConfig: any): boolean { +function toPgPool(poolOrConfig: any): Pool { + if (quacksLikePgPool(poolOrConfig)) { + // If it is already a `Pool`, just use it. + return poolOrConfig; + } + + return new Pool( + typeof poolOrConfig === 'string' + ? // Otherwise if it is a string, let us parse it to get a config to + // create a `Pool`. + parsePgConnectionString(poolOrConfig) + : // Finally, it must just be a config itself. If it is undefined, we + // will just use an empty config and let the defaults take over. + poolOrConfig || {}, + ); +} + +// tslint:disable-next-line no-any +function quacksLikePgPool(pgConfig: any): pgConfig is Pool { + if (pgConfig instanceof Pool) return true; + // A diagnosis of exclusion if (!pgConfig || typeof pgConfig !== 'object') return false; if (constructorName(pgConfig) !== 'Pool' && constructorName(pgConfig) !== 'BoundPool')