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.

Sign up for GitHub

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

Fix query and path params not keeping the coerced types #262

Closed
wants to merge 10 commits into from
Closed

Fix query and path params not keeping the coerced types #262

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4ac6138
fix: :bug: Fix queryParams not keeping the coerced types
Fetchinator7 Dec 1, 2021
ff07b43
refactor: :recycle: Rename `ParsedRequestKeys` to `ParamValue`
Fetchinator7 Feb 12, 2022
1ab18a7
fix: :bug: Fix query and path params not keeping coerced types (ajv c…
Fetchinator7 Feb 12, 2022
5a3eb17
refactor: :fire: Remove useless default value
Fetchinator7 Feb 12, 2022
b191215
Merge branch 'master' into fix-req-query-parsing
anttiviljami Feb 20, 2022
957df3b
style: :white_check_mark: Fix lint style errors
Fetchinator7 Feb 21, 2022
d8c38ac
fix: :white_check_mark: Fix path params not being coerced
Fetchinator7 Feb 21, 2022
e208cdd
fix: :white_check_mark: Fix header params not being coerced
Fetchinator7 Feb 22, 2022
9608fd4
Merge branch 'master' of https://github.com/anttiviljami/openapi-back…
Fetchinator7 May 15, 2022
930c194
fix: :art: Run `lint` and `prettier` scripts
Fetchinator7 May 15, 2022
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
93 changes: 85 additions & 8 deletions src/router.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ const responses: OpenAPIV3_1.ResponsesObject = {
200: { description: 'ok' },
};

const ownerHeader: OpenAPIV3_1.ParameterObject = {
in: 'header',
name: 'has-owner',
required: false,
schema: {
type: 'boolean',
},
};

const pathId: OpenAPIV3_1.ParameterObject = {
name: 'id',
in: 'path',
Expand Down Expand Up @@ -36,6 +45,14 @@ const queryLimit: OpenAPIV3_1.ParameterObject = {
},
};

const queryOwnerFilter: OpenAPIV3_1.ParameterObject = {
name: 'hasOwner',
in: 'query',
schema: {
type: 'boolean',
},
};

const queryFilter: OpenAPIV3_1.ParameterObject = {
name: 'filter',
in: 'query',
Expand Down Expand Up @@ -81,7 +98,7 @@ const definition: OpenAPIV3_1.Document = {
operationId: 'createPet',
responses,
},
parameters: [queryLimit, queryFilter],
parameters: [queryLimit, queryFilter, queryOwnerFilter],
},
'/pets/{id}': {
get: {
Expand Down Expand Up @@ -114,7 +131,7 @@ const definition: OpenAPIV3_1.Document = {
operationId: 'getPetHobbies',
responses,
},
parameters: [pathId, hobbyId],
parameters: [pathId, hobbyId, queryLimit, ownerHeader],
},
'/pets/meta': {
get: {
Expand Down Expand Up @@ -149,7 +166,7 @@ describe('OpenAPIRouter', () => {
expect(parsedRequest.requestBody).toEqual(payload);
});

test('parses request body passed as JSON', () => {
test('parses request body passed as JSON string', () => {
const payload = { horse: 1 };
const request = { path: '/pets', method: 'post', body: JSON.stringify(payload), headers };

Expand All @@ -163,7 +180,7 @@ describe('OpenAPIRouter', () => {
const operation = api.getOperation('getPetById')!;

const parsedRequest = api.parseRequest(request, operation);
expect(parsedRequest.params).toEqual({ id: '123' });
expect(parsedRequest.params).toEqual({ id: 123 });
});

test('parses query string from path prop', () => {
Expand Down Expand Up @@ -201,12 +218,56 @@ describe('OpenAPIRouter', () => {
expect(parsedRequest.query.filter).toEqual(filterValue);
});

test('parses query string without type coercion when `limit` type=integer with float input', () => {
const request = { path: '/pets?limit=10.3', method: 'get', headers, parameters: [queryLimit] };
const operation = api.getOperation('getPets');

const parsedRequest = api.parseRequest(request, operation);
// the limit schema is an integer so this would cause an input validation error and not parse correctly and thus remain a string.
expect(parsedRequest.query).toEqual({ limit: '10.3' });
});

test('parses query string when `limit` type=integer', () => {
const request = { path: '/pets?limit=10', method: 'get', headers, parameters: [queryLimit] };
const operation = api.getOperation('getPets');

const parsedRequest = api.parseRequest(request, operation);
expect(parsedRequest.query).toEqual({ limit: 10 });
});

test('parses query string arrays', () => {
const request = { path: '/pets?limit=10&limit=20', method: 'get', headers };
const parsedRequest = api.parseRequest(request);
expect(parsedRequest.query).toEqual({ limit: ['10', '20'] });
});

test('parses query string to boolean when `hasOwner` type=boolean', () => {
const request = { path: '/pets?hasOwner=false', method: 'get', parameters: [queryOwnerFilter], headers };
const operation = api.getOperation('getPets');

const parsedRequest = api.parseRequest(request, operation);
expect(parsedRequest.query).toEqual({ hasOwner: false });
});

test('parses query string arrays as integers when style=form, explode=false, type=integer', () => {
const request = { path: '/pets?limit=10,20', method: 'get', headers, parameters: [queryLimit] };
const operation = api.getOperation('createPet')!;
operation.parameters = [
{
in: 'query',
name: 'limit',
style: 'form',
explode: false,
schema: {
type: 'integer',
},
},
];

const parsedRequest = api.parseRequest(request, operation);
expect(parsedRequest.query).toEqual({ limit: [10, 20] });
});

test('parses query string arrays when style=form, explode=false', () => {
const request = { path: '/pets?limit=10,20', method: 'get', headers };
const operation = api.getOperation('createPet')!;
Expand Down Expand Up @@ -270,6 +331,21 @@ describe('OpenAPIRouter', () => {
const parsedRequest = api.parseRequest(request, operation);
expect(parsedRequest.query).toEqual({ limit: ['10', '20'] });
});

test('parses path parameters and query parameters /pets/{id}/hobbies/{hobbyId}?limit', async () => {
const request = {
path: '/pets/1/hobbies/3?limit=5',
method: 'get',
headers: { ...headers, 'has-owner': 'false' },
parameters: [pathId, hobbyId, queryLimit, ownerHeader],
};
const operation = api.getOperation('getPetHobbies');

const parsedRequest = api.parseRequest(request, operation);
expect(parsedRequest.headers).toEqual({ ...headers, 'has-owner': false });
expect(parsedRequest.query).toEqual({ limit: 5 });
expect(parsedRequest.params).toEqual({ hobbyId: 3, id: 1 });
});
});

describe('.matchOperation', () => {
Expand Down Expand Up @@ -456,7 +532,7 @@ describe('OpenAPIBackend', () => {
expect(dummyHandlers['getPetById']).toBeCalled();

const contextArg = dummyHandlers['getPetById'].mock.calls.slice(-1)[0][0];
expect(contextArg.request).toMatchObject({ method: 'get', path: '/pets/1', params: { id: '1' }, headers });
expect(contextArg.request).toMatchObject({ method: 'get', path: '/pets/1', params: { id: 1 }, headers });
expect(contextArg.operation.operationId).toEqual('getPetById');
expect(contextArg.validation.errors).toBeFalsy();
});
Expand All @@ -467,7 +543,7 @@ describe('OpenAPIBackend', () => {
expect(dummyHandlers['updatePetById']).toBeCalled();

const contextArg = dummyHandlers['updatePetById'].mock.calls.slice(-1)[0][0];
expect(contextArg.request).toMatchObject({ method: 'patch', path: '/pets/1', params: { id: '1' }, headers });
expect(contextArg.request).toMatchObject({ method: 'patch', path: '/pets/1', params: { id: 1 }, headers });
expect(contextArg.operation.operationId).toEqual('updatePetById');
expect(contextArg.validation.errors).toBeFalsy();
});
Expand All @@ -487,7 +563,7 @@ describe('OpenAPIBackend', () => {
expect(dummyHandlers['notImplemented']).toBeCalled();

const contextArg = dummyHandlers['notImplemented'].mock.calls.slice(-1)[0][0];
expect(contextArg.request).toMatchObject({ method: 'delete', path: '/pets/1', params: { id: '1' }, headers });
expect(contextArg.request).toMatchObject({ method: 'delete', path: '/pets/1', params: { id: 1 }, headers });
expect(contextArg.operation.operationId).toEqual('deletePetById');
expect(contextArg.validation.errors).toBeFalsy();
});
Expand All @@ -509,7 +585,8 @@ describe('OpenAPIBackend', () => {
expect(dummyHandlers['getPets']).toBeCalled();

const contextArg = dummyHandlers['getPets'].mock.calls.slice(-1)[0][0];
expect(contextArg.request).toMatchObject({ method: 'get', path: '/pets/', query: { limit: '10' }, headers });
// the limit schema is an integer so it should be parsed as the number 10
expect(contextArg.request).toMatchObject({ method: 'get', path: '/pets/', query: { limit: 10 }, headers });
expect(contextArg.operation.operationId).toEqual('getPets');
expect(contextArg.validation.errors).toBeFalsy();
});
Expand Down
116 changes: 90 additions & 26 deletions src/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import * as cookie from 'cookie';
import { parse as parseQuery } from 'qs';
import { Parameters } from 'bath-es5/_/types';
import { PickVersionElement } from './backend';
import Ajv from 'ajv';

// alias Document to OpenAPIV3_1.Document
type Document = OpenAPIV3_1.Document | OpenAPIV3.Document;
Expand All @@ -30,29 +31,31 @@ export type Operation<D extends Document = Document> = PickVersionElement<
method: string;
};

type ParamValue = string | number | boolean;

export interface Request {
method: string;
path: string;
headers: {
[key: string]: string | string[];
[key: string]: ParamValue | ParamValue[];
};
query?:
| {
[key: string]: string | string[];
[key: string]: ParamValue | ParamValue[];
}
| string;
body?: any;
}

export interface ParsedRequest extends Request {
params: {
[key: string]: string | string[];
[key: string]: ParamValue | ParamValue[];
};
cookies: {
[key: string]: string | string[];
[key: string]: ParamValue | ParamValue[];
};
query: {
[key: string]: string | string[];
[key: string]: ParamValue | ParamValue[];
};
requestBody: any;
}
Expand Down Expand Up @@ -278,15 +281,15 @@ export class OpenAPIRouter<D extends Document = Document> {
}

// header keys are converted to lowercase, so Content-Type becomes content-type
const headers = _.mapKeys(req.headers, (val, header) => header.toLowerCase());
let headers = _.mapKeys(req.headers, (val, header) => header.toLowerCase());

// parse cookie from headers
const cookieHeader = headers['cookie'];
const cookies = cookie.parse(_.flatten([cookieHeader]).join('; '));

// parse query
const queryString = typeof req.query === 'string' ? req.query.replace('?', '') : req.path.split('?')[1];
const query = typeof req.query === 'object' ? _.cloneDeep(req.query) : parseQuery(queryString);
let query = typeof req.query === 'object' ? _.cloneDeep(req.query) : parseQuery(queryString);

// normalize
req = this.normalizeRequest(req);
Expand All @@ -300,30 +303,91 @@ export class OpenAPIRouter<D extends Document = Document> {
const pathParams = bath(operation.path);
params = pathParams.params(normalizedPath) || {};
// parse query parameters with specified style for parameter
for (const queryParam in query) {
if (query[queryParam]) {
const parameter = operation.parameters?.find(
(param) => !('$ref' in param) && param?.in === 'query' && param?.name === queryParam,
) as PickVersionElement<D, OpenAPIV3.ParameterObject, OpenAPIV3_1.ParameterObject>;

if (parameter) {
if (parameter.content && parameter.content['application/json']) {
query[queryParam] = JSON.parse(query[queryParam]);
} else if (parameter.explode === false && queryString) {
let commaQueryString = queryString.replace(/\%2C/g, ',');
if (parameter.style === 'spaceDelimited') {
commaQueryString = commaQueryString.replace(/\ /g, ',').replace(/\%20/g, ',');

/**
* Coerce the input parameters to be the correct type (if specified, otherwise it defaults to a string).
* @param inputObj The object with input parameters to coerce.
* @param location If the parameter is in the `path` or `query`
*/
const coerceInputs = (inputObj: any, location: 'query' | 'path' | 'header') => {
const coerced: any = _.cloneDeep(inputObj);
for (const queryParam in coerced) {
if (coerced[queryParam]) {
const parameter = operation.parameters?.find(
(param) => !('$ref' in param) && param?.in === location && param?.name === queryParam,
) as PickVersionElement<D, OpenAPIV3.ParameterObject, OpenAPIV3_1.ParameterObject>;

if (parameter) {
if (parameter.content && parameter.content['application/json']) {
// JSON.parse handles parsing to correct data types, no additional logic necessary.
coerced[queryParam] = JSON.parse(coerced[queryParam]);
} else if (parameter.explode === false && queryString) {
let commaQueryString = queryString.replace(/\%2C/g, ',');
if (parameter.style === 'spaceDelimited') {
commaQueryString = commaQueryString.replace(/\ /g, ',').replace(/\%20/g, ',');
}
if (parameter.style === 'pipeDelimited') {
commaQueryString = commaQueryString.replace(/\|/g, ',').replace(/\%7C/g, ',');
}
// use comma parsing e.g. &a=1,2,3
const commaParsed = parseQuery(commaQueryString, { comma: true });
coerced[queryParam] = commaParsed[queryParam];
}
if (parameter.style === 'pipeDelimited') {
commaQueryString = commaQueryString.replace(/\|/g, ',').replace(/\%7C/g, ',');

// If a schema is specified use Ajv type coercion.
if (parameter.schema) {
const ajv = new Ajv({ coerceTypes: 'array' });
/**
* The full representation of the desired schema.
* This allows for short-hand inputs of types like `type: integer`
* then we expand it here so Ajv parses it correctly, otherwise we'll get an error.
*/
let parseSchema: PickVersionElement<D, OpenAPIV3.SchemaObject, OpenAPIV3_1.SchemaObject>;
let queryData = coerced[queryParam];
const isArray = Array.isArray(queryData);
/** The specified schema type. */
const specifiedSchemaType = (
parameter.schema as PickVersionElement<D, OpenAPIV3.SchemaObject, OpenAPIV3_1.SchemaObject>
)?.type;
// Even if the input is an array but not defined as an array still parse it as an array anyway.
if (isArray && specifiedSchemaType !== 'array') {
parseSchema = {
type: 'array',
items: {
...parameter.schema,
},
};
} else {
// Expand short-hand to full schema.
parseSchema = {
type: 'object',
properties: {
[queryParam]: parameter.schema,
},
};
queryData = { [queryParam]: coerced[queryParam] };
}
const coerceData = ajv.compile(parseSchema);
// Set `queryData` to the type coerced value(s).
coerceData(queryData);
if (isArray && specifiedSchemaType !== 'array') {
// Set the query array to the type-coerced array.
coerced[queryParam] = queryData;
} else {
// Set the query to the type-coerced value.
coerced[queryParam] = queryData[queryParam];
}
}
// use comma parsing e.g. &a=1,2,3
const commaParsed = parseQuery(commaQueryString, { comma: true });
query[queryParam] = commaParsed[queryParam];
}
}
}
}
return coerced;
};

// Coerce the inputs to be the correct type and overwrite the request with the correct values.
params = coerceInputs(params, 'path');
query = coerceInputs(query, 'query');
headers = coerceInputs(headers, 'header');
}

return {
Expand Down
Loading