From b570c0d29221f98f9b9182b10f72b470739e86f9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 24 Oct 2021 17:08:30 +0000 Subject: [PATCH 1/4] chore(deps-dev): bump @types/busboy from 0.3.0 to 0.3.1 Bumps [@types/busboy](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/busboy) from 0.3.0 to 0.3.1. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/busboy) --- updated-dependencies: - dependency-name: "@types/busboy" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 45759ef4a5..86f675449e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5336,9 +5336,9 @@ } }, "@types/busboy": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/@types/busboy/-/busboy-0.3.0.tgz", - "integrity": "sha512-4NEQcEn42ofSV9OuSaxqxM9CapPrcRMoezr7dCHfXEBUIm86QGmDv28rsi1PExgUEAMqZyN5YaMKwLFUfRnT0Q==", + "version": "0.3.1", + "resolved": "https://registry.npmjs.org/@types/busboy/-/busboy-0.3.1.tgz", + "integrity": "sha512-8BPLNy4x+7lbTOGkAyUIZrrPEZ7WzbO7YlVGMf9EZi9J9mqILEkYbt/kgVWQ7fizOISo1hM/7cAsWVTa7EhQDg==", "dev": true, "requires": { "@types/node": "*" diff --git a/package.json b/package.json index 24b7316955..559ea57444 100644 --- a/package.json +++ b/package.json @@ -165,7 +165,7 @@ "@opengovsg/mockpass": "^2.7.9", "@types/bcrypt": "^5.0.0", "@types/bluebird": "^3.5.36", - "@types/busboy": "^0.3.0", + "@types/busboy": "^0.3.1", "@types/compression": "^1.7.2", "@types/connect-datadog": "0.0.6", "@types/convict": "^6.1.1", From 2dd1a2c598aef9eee7c4a7892a19cf3c032be08c Mon Sep 17 00:00:00 2001 From: Kar Rui Lau Date: Mon, 25 Oct 2021 11:09:42 +0800 Subject: [PATCH 2/4] fix: update busboy namespace back to lowercased variant --- .../submission/email-submission/email-submission.receiver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/modules/submission/email-submission/email-submission.receiver.ts b/src/app/modules/submission/email-submission/email-submission.receiver.ts index 9f9c38c911..dddd400f4d 100644 --- a/src/app/modules/submission/email-submission/email-submission.receiver.ts +++ b/src/app/modules/submission/email-submission/email-submission.receiver.ts @@ -26,7 +26,7 @@ const logger = createLoggerWithLabel(module) */ export const createMultipartReceiver = ( headers: IncomingHttpHeaders, -): Result => { +): Result => { try { const busboy = new Busboy({ headers, @@ -55,7 +55,7 @@ export const createMultipartReceiver = ( * @param busboy Busboy receiver object */ export const configureMultipartReceiver = ( - busboy: Busboy.Busboy, + busboy: busboy.Busboy, ): ResultAsync => { const logMeta = { action: 'configureMultipartReceiver', From a1f45b98b5e9f5fc345adf1239d7659038e7e32e Mon Sep 17 00:00:00 2001 From: Kar Rui Lau Date: Mon, 25 Oct 2021 11:27:08 +0800 Subject: [PATCH 3/4] feat: add typeguard for busboy headers --- .../email-submission.receiver.ts | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/app/modules/submission/email-submission/email-submission.receiver.ts b/src/app/modules/submission/email-submission/email-submission.receiver.ts index dddd400f4d..e8d286f7a0 100644 --- a/src/app/modules/submission/email-submission/email-submission.receiver.ts +++ b/src/app/modules/submission/email-submission/email-submission.receiver.ts @@ -1,4 +1,4 @@ -import Busboy from 'busboy' +import Busboy, { BusboyHeaders } from 'busboy' import { IncomingHttpHeaders } from 'http' import { err, ok, Result, ResultAsync } from 'neverthrow' @@ -20,13 +20,30 @@ import { const logger = createLoggerWithLabel(module) +const isBusboyHeaders = ( + headers: IncomingHttpHeaders, +): headers is BusboyHeaders => { + return !!headers['content-type'] +} + /** * Initialises a Busboy object to receive the submission stream * @param headers HTTP request headers */ export const createMultipartReceiver = ( headers: IncomingHttpHeaders, -): Result => { +): Result => { + if (!isBusboyHeaders(headers)) { + logger.error({ + message: "Busboy cannot be init due to missing headers['content-type']", + meta: { + action: 'createMultipartReceiver', + headers, + }, + }) + return err(new InitialiseMultipartReceiverError()) + } + try { const busboy = new Busboy({ headers, @@ -55,7 +72,7 @@ export const createMultipartReceiver = ( * @param busboy Busboy receiver object */ export const configureMultipartReceiver = ( - busboy: busboy.Busboy, + busboy: Busboy.Busboy, ): ResultAsync => { const logMeta = { action: 'configureMultipartReceiver', From 7252b4708f146f2664ed5f45d5c50d5294bf5181 Mon Sep 17 00:00:00 2001 From: Kar Rui Lau Date: Mon, 25 Oct 2021 15:05:45 +0800 Subject: [PATCH 4/4] test(email-submission): add test for typeguard in busboy headers --- .../email-submission.receiver.spec.ts | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/app/modules/submission/email-submission/__tests__/email-submission.receiver.spec.ts b/src/app/modules/submission/email-submission/__tests__/email-submission.receiver.spec.ts index 8350c28d35..d8516aa8b8 100644 --- a/src/app/modules/submission/email-submission/__tests__/email-submission.receiver.spec.ts +++ b/src/app/modules/submission/email-submission/__tests__/email-submission.receiver.spec.ts @@ -2,7 +2,7 @@ import Busboy from 'busboy' import FormData from 'form-data' import { createReadStream, readFileSync } from 'fs' import { IncomingHttpHeaders } from 'http' -import { pick } from 'lodash' +import { omit, pick } from 'lodash' import { mocked } from 'ts-jest/utils' import { @@ -22,12 +22,15 @@ jest.mock('busboy') const MockBusboy = mocked(Busboy, true) const RealBusboy = jest.requireActual('busboy') as typeof Busboy -const MOCK_HEADERS = { key: 'value' } +const MOCK_HEADERS: IncomingHttpHeaders = { + 'content-type': 'anything', + key: 'value', +} const MOCK_BUSBOY_ON = jest.fn().mockReturnThis() const MOCK_BUSBOY = { on: MOCK_BUSBOY_ON, -} as unknown as busboy.Busboy +} as unknown as Busboy.Busboy const VALID_FILE_PATH = 'tests/unit/backend/resources/' const VALID_FILENAME_1 = 'valid.txt' @@ -40,13 +43,15 @@ const VALID_FILE_CONTENT_2 = readFileSync( ) describe('email-submission.receiver', () => { + beforeEach(() => { + MockBusboy.mockReset() + }) describe('createMultipartReceiver', () => { it('should call Busboy constructor with the correct params', () => { MockBusboy.mockReturnValueOnce(MOCK_BUSBOY) - const result = EmailSubmissionReceiver.createMultipartReceiver( - MOCK_HEADERS as IncomingHttpHeaders, - ) + const result = + EmailSubmissionReceiver.createMultipartReceiver(MOCK_HEADERS) expect(MockBusboy).toHaveBeenCalledWith({ headers: MOCK_HEADERS, @@ -58,14 +63,25 @@ describe('email-submission.receiver', () => { expect(result._unsafeUnwrap()).toEqual(MOCK_BUSBOY) }) + it('should return error headers are missing content-type key-value', () => { + const result = EmailSubmissionReceiver.createMultipartReceiver( + omit(MOCK_HEADERS, 'content-type'), + ) + + // Should have failed type guard and not have been called. + expect(MockBusboy).not.toHaveBeenCalled() + expect(result._unsafeUnwrapErr()).toEqual( + new InitialiseMultipartReceiverError(), + ) + }) + it('should return error when busboy constructor errors', () => { MockBusboy.mockImplementationOnce(() => { throw new Error() }) - const result = EmailSubmissionReceiver.createMultipartReceiver( - MOCK_HEADERS as IncomingHttpHeaders, - ) + const result = + EmailSubmissionReceiver.createMultipartReceiver(MOCK_HEADERS) expect(MockBusboy).toHaveBeenCalledWith({ headers: MOCK_HEADERS,