From f63ad78082a6157d3cc9b09882547c2bc6715b61 Mon Sep 17 00:00:00 2001 From: aramissennyeydd Date: Sun, 7 Jul 2024 16:37:12 -0400 Subject: [PATCH] more fixes and validating with search-backend Signed-off-by: aramissennyeydd --- packages/backend-openapi-utils/package.json | 1 + .../backend-openapi-utils/src/proxy/setup.ts | 17 +++++++---- .../schema/request-body-validation.test.ts | 28 ++++++++++++++++++- .../src/schema/request-body-validation.ts | 10 +++++-- .../src/schema/response-body-validation.ts | 21 ++++++++++---- .../backend-openapi-utils/src/testUtils.ts | 4 +-- yarn.lock | 8 ++++++ 7 files changed, 72 insertions(+), 17 deletions(-) diff --git a/packages/backend-openapi-utils/package.json b/packages/backend-openapi-utils/package.json index 5fb1f98e1b..04653c5b32 100644 --- a/packages/backend-openapi-utils/package.json +++ b/packages/backend-openapi-utils/package.json @@ -44,6 +44,7 @@ "express": "^4.17.1", "express-openapi-validator": "^5.0.4", "express-promise-router": "^4.1.0", + "get-port": "^7.1.0", "json-schema-to-ts": "^3.0.0", "lodash": "^4.17.21", "mockttp": "^3.13.0", diff --git a/packages/backend-openapi-utils/src/proxy/setup.ts b/packages/backend-openapi-utils/src/proxy/setup.ts index 8f24aa2512..da84d44afc 100644 --- a/packages/backend-openapi-utils/src/proxy/setup.ts +++ b/packages/backend-openapi-utils/src/proxy/setup.ts @@ -16,6 +16,8 @@ import * as mockttp from 'mockttp'; import { OpenApiProxyValidator } from '../schema/validation'; +import getPort from 'get-port'; +import { Server } from 'http'; export class Proxy { server: mockttp.Mockttp; @@ -25,6 +27,8 @@ export class Proxy { mockttp.CompletedResponse >(); validator: OpenApiProxyValidator; + public forwardTo: { port: number } = { port: 0 }; + express: { server: Server | undefined } = { server: undefined }; constructor() { this.server = mockttp.getLocal(); this.validator = new OpenApiProxyValidator(); @@ -32,9 +36,10 @@ export class Proxy { async setup() { await this.server.start(); + this.forwardTo.port = await getPort(); this.server .forAnyRequest() - .thenForwardTo(`http://localhost:${process.env.PORT}`); + .thenForwardTo(`http://localhost:${this.forwardTo.port}`); await this.server.on('request', request => { this.#openRequests[request.id] = request; }); @@ -53,10 +58,9 @@ export class Proxy { }); } - async initialize() { - await this.validator.initialize( - `http://localhost:${process.env.PORT}/openapi.json`, - ); + async initialize(url: string, server: Server) { + await this.validator.initialize(`${url}/openapi.json`); + this.express.server = server; } stop() { @@ -64,6 +68,9 @@ export class Proxy { throw new Error('There are still open requests'); } this.server.stop(); + + // If this isn't expressly closed, it will cause a jest memory leak warning. + this.express.server?.close(); } get url() { diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts index df8b33402c..807eaa4569 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.test.ts @@ -19,7 +19,12 @@ import { RequestBodyParser } from './request-body-validation'; import Ajv from 'ajv'; import { Operation, RequestParser } from './types'; import _ from 'lodash'; -import { OperationObject, RequestBodyObject } from 'openapi3-ts'; +import { + ContentObject, + MediaTypeObject, + OperationObject, + RequestBodyObject, +} from 'openapi3-ts'; import { JsonObject } from '@backstage/types'; const ajv = new Ajv(); @@ -79,4 +84,25 @@ describe('request body', () => { `"["POST /api/search"] No request body found for /api/search"`, ); }); + + it('should throw error if request body is not application/json', async () => { + const request = toRequest({}, { 'content-type': 'text/plain' }); + await expect( + parser.parse(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"["POST /api/search"] Content type is not application/json"`, + ); + }); + + it('should NOT throw error if request body is not just application/json', async () => { + (schema.requestBody.content as ContentObject)[ + 'application/json; charset=utf-8' + ] = schema.requestBody.content['application/json'] as MediaTypeObject; + delete (schema.requestBody.content as ContentObject)['application/json']; + parser = new RequestBodyParser(operation, { + ajv, + }); + const request = toRequest({}); + expect(await parser.parse(request)).toEqual({}); + }); }); diff --git a/packages/backend-openapi-utils/src/schema/request-body-validation.ts b/packages/backend-openapi-utils/src/schema/request-body-validation.ts index f7681f01c2..0075bae641 100644 --- a/packages/backend-openapi-utils/src/schema/request-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/request-body-validation.ts @@ -77,13 +77,17 @@ export class RequestBodyParser 'No content found in request body', ); } - if (!requestBody!.content['application/json']) { + const contentTypes = requestBody!.content; + const jsonContentType = Object.keys(contentTypes).find(contentType => + contentType.split(';').includes('application/json'), + ); + if (!jsonContentType) { throw new OperationError( this.operation, 'No application/json content type found in request body', ); } - const schema = requestBody!.content['application/json'].schema; + const schema = requestBody!.content[jsonContentType].schema; if (!schema) { throw new OperationError( this.operation, @@ -111,7 +115,7 @@ export class RequestBodyParser const contentType = request.headers.get('content-type') || 'application/json'; - if (contentType !== 'application/json') { + if (!contentType.split(';').includes('application/json')) { throw new OperationError( this.operation, 'Content type is not application/json', diff --git a/packages/backend-openapi-utils/src/schema/response-body-validation.ts b/packages/backend-openapi-utils/src/schema/response-body-validation.ts index 1a2a475afc..71a4512e75 100644 --- a/packages/backend-openapi-utils/src/schema/response-body-validation.ts +++ b/packages/backend-openapi-utils/src/schema/response-body-validation.ts @@ -61,15 +61,20 @@ export class ResponseBodyParser this.ajv = options.ajv; const responseSchemas = operation.schema.responses; for (const [statusCode, schema] of Object.entries(responseSchemas)) { - if (!schema.content) { + const contentTypes = schema.content; + if (!contentTypes) { // Skip responses without content, eg 204 No Content. continue; - } else if (!schema.content['application/json']) { + } + const jsonContentType = Object.keys(contentTypes).find(contentType => + contentType.split(';').includes('application/json'), + ); + if (!jsonContentType) { throw new OperationError( this.operation, `No application/json content type found in response for status code ${statusCode}`, ); - } else if ('$ref' in schema.content['application/json'].schema) { + } else if ('$ref' in contentTypes[jsonContentType].schema) { throw new OperationError( this.operation, 'Reference objects are not supported', @@ -97,21 +102,25 @@ export class ResponseBodyParser ); } - if (!responseSchema?.content && body?.length) { + const contentTypes = responseSchema.content; + if (!contentTypes && body?.length) { throw new OperationResponseError( this.operation, response, 'Received a body but no schema was found', ); } - if (!responseSchema?.content!['application/json']) { + const jsonContentType = Object.keys(contentTypes ?? {}).find(contentType => + contentType.split(';').includes('application/json'), + ); + if (!jsonContentType) { throw new OperationResponseError( this.operation, response, 'No application/json content type found in response', ); } - const schema = responseSchema.content!['application/json'].schema; + const schema = responseSchema.content![jsonContentType].schema; // This is a bit of type laziness. Ideally, this would be a type-narrowing function, but I wasn't able to get the types to work. if (!schema) { throw new OperationError(this.operation, 'No schema found in response'); diff --git a/packages/backend-openapi-utils/src/testUtils.ts b/packages/backend-openapi-utils/src/testUtils.ts index f96a0b1b76..d93029fa8e 100644 --- a/packages/backend-openapi-utils/src/testUtils.ts +++ b/packages/backend-openapi-utils/src/testUtils.ts @@ -28,8 +28,8 @@ afterAll(() => { }); export async function wrapServer(app: Express): Promise { - const server = app.listen(+process.env.PORT!); - await proxy.initialize(); + const server = app.listen(proxy.forwardTo.port); + await proxy.initialize(`http://localhost:${proxy.forwardTo.port}`, server); return { ...server, address: () => new URL(proxy.url) } as any; } diff --git a/yarn.lock b/yarn.lock index bcc6a951fa..bf267d84fd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3750,6 +3750,7 @@ __metadata: express: ^4.17.1 express-openapi-validator: ^5.0.4 express-promise-router: ^4.1.0 + get-port: ^7.1.0 json-schema-to-ts: ^3.0.0 lodash: ^4.17.21 mockttp: ^3.13.0 @@ -28304,6 +28305,13 @@ __metadata: languageName: node linkType: hard +"get-port@npm:^7.1.0": + version: 7.1.0 + resolution: "get-port@npm:7.1.0" + checksum: f4d23b43026124007663a899578cc87ff37bfcf645c5c72651e9810ebafc759857784e409fb8e0ada9b90e5c5db089b0ae2f5f6b49fba1ce2e0aff86094ab17d + languageName: node + linkType: hard + "get-stdin@npm:^9.0.0": version: 9.0.0 resolution: "get-stdin@npm:9.0.0"