From dee6c28db977e84d61e743341c9efe6607b3afcf Mon Sep 17 00:00:00 2001 From: Abhinav Kumar Date: Tue, 3 Dec 2024 00:27:43 +0530 Subject: [PATCH] regression: undefined fileBuffer in getUploadFormData when onFile is called before onField (#34091) --- .../api/server/lib/getUploadFormData.spec.ts | 178 ++++++++++++++++++ .../app/api/server/lib/getUploadFormData.ts | 21 +-- 2 files changed, 188 insertions(+), 11 deletions(-) create mode 100644 apps/meteor/app/api/server/lib/getUploadFormData.spec.ts diff --git a/apps/meteor/app/api/server/lib/getUploadFormData.spec.ts b/apps/meteor/app/api/server/lib/getUploadFormData.spec.ts new file mode 100644 index 000000000000..dc7afb77bd19 --- /dev/null +++ b/apps/meteor/app/api/server/lib/getUploadFormData.spec.ts @@ -0,0 +1,178 @@ +import { Readable } from 'stream'; + +import { expect } from 'chai'; +import type { Request } from 'express'; + +import { getUploadFormData } from './getUploadFormData'; + +const createMockRequest = ( + fields: Record, + file?: { + fieldname: string; + filename: string; + content: string | Buffer; + mimetype?: string; + }, +): Readable & { headers: Record } => { + const boundary = '----WebKitFormBoundary7MA4YWxkTrZu0gW'; + const parts: string[] = []; + + if (file) { + parts.push( + `--${boundary}`, + `Content-Disposition: form-data; name="${file.fieldname}"; filename="${file.filename}"`, + `Content-Type: ${file.mimetype || 'application/octet-stream'}`, + '', + file.content.toString(), + ); + } + + for (const [name, value] of Object.entries(fields)) { + parts.push(`--${boundary}`, `Content-Disposition: form-data; name="${name}"`, '', value); + } + + parts.push(`--${boundary}--`); + + const mockRequest: any = new Readable({ + read() { + this.push(Buffer.from(parts.join('\r\n'))); + this.push(null); + }, + }); + + mockRequest.headers = { + 'content-type': `multipart/form-data; boundary=${boundary}`, + }; + + return mockRequest as Readable & { headers: Record }; +}; + +describe('getUploadFormData', () => { + it('should successfully parse a single file upload and fields', async () => { + const mockRequest = createMockRequest( + { fieldName: 'fieldValue' }, + { + fieldname: 'fileField', + filename: 'test.txt', + content: 'Hello, this is a test file!', + mimetype: 'text/plain', + }, + ); + + const result = await getUploadFormData({ request: mockRequest as Request }, { field: 'fileField' }); + + expect(result).to.deep.include({ + fieldname: 'fileField', + filename: 'test.txt', + mimetype: 'text/plain', + fields: { fieldName: 'fieldValue' }, + }); + + expect(result.fileBuffer).to.not.be.undefined; + expect(result.fileBuffer.toString()).to.equal('Hello, this is a test file!'); + }); + it('should parse a file upload with multiple additional fields', async () => { + const mockRequest = createMockRequest( + { + fieldName: 'fieldValue', + extraField1: 'extraValue1', + extraField2: 'extraValue2', + }, + { + fieldname: 'fileField', + filename: 'test_with_fields.txt', + content: 'This file has additional fields!', + mimetype: 'text/plain', + }, + ); + + const result = await getUploadFormData({ request: mockRequest as Request }, { field: 'fileField' }); + + expect(result).to.deep.include({ + fieldname: 'fileField', + filename: 'test_with_fields.txt', + mimetype: 'text/plain', + fields: { + fieldName: 'fieldValue', + extraField1: 'extraValue1', + extraField2: 'extraValue2', + }, + }); + + expect(result.fileBuffer).to.not.be.undefined; + expect(result.fileBuffer.toString()).to.equal('This file has additional fields!'); + }); + + it('should handle a file upload when fileOptional is true', async () => { + const mockRequest = createMockRequest( + { fieldName: 'fieldValue' }, + { + fieldname: 'fileField', + filename: 'optional.txt', + content: 'This file is optional!', + mimetype: 'text/plain', + }, + ); + + const result = await getUploadFormData({ request: mockRequest as Request }, { fileOptional: true }); + + expect(result).to.deep.include({ + fieldname: 'fileField', + filename: 'optional.txt', + mimetype: 'text/plain', + fields: { fieldName: 'fieldValue' }, + }); + + expect(result.fileBuffer).to.not.be.undefined; + expect(result.fileBuffer?.toString()).to.equal('This file is optional!'); + }); + + it('should throw an error when no file is uploaded and fileOptional is false', async () => { + const mockRequest = createMockRequest({ fieldName: 'fieldValue' }); + + try { + await getUploadFormData({ request: mockRequest as Request }, { fileOptional: false }); + throw new Error('Expected function to throw'); + } catch (error) { + expect((error as Error).message).to.equal('[No file uploaded]'); + } + }); + + it('should return fields without errors when no file is uploaded but fileOptional is true', async () => { + const mockRequest = createMockRequest({ fieldName: 'fieldValue' }); // No file + + const result = await getUploadFormData({ request: mockRequest as Request }, { fileOptional: true }); + + expect(result).to.deep.equal({ + fields: { fieldName: 'fieldValue' }, + file: undefined, + fileBuffer: undefined, + fieldname: undefined, + filename: undefined, + encoding: undefined, + mimetype: undefined, + }); + }); + + it('should reject an oversized file', async () => { + const mockRequest = createMockRequest( + {}, + { + fieldname: 'fileField', + filename: 'large.txt', + content: 'x'.repeat(1024 * 1024 * 2), // 2 MB file + mimetype: 'text/plain', + }, + ); + + try { + await getUploadFormData( + { request: mockRequest as Request }, + { sizeLimit: 1024 * 1024 }, // 1 MB limit + ); + throw new Error('Expected function to throw'); + } catch (error) { + expect((error as Error).message).to.equal('[error-file-too-large]'); + } + }); +}); diff --git a/apps/meteor/app/api/server/lib/getUploadFormData.ts b/apps/meteor/app/api/server/lib/getUploadFormData.ts index 1630afe8cae2..93ceafdde92f 100644 --- a/apps/meteor/app/api/server/lib/getUploadFormData.ts +++ b/apps/meteor/app/api/server/lib/getUploadFormData.ts @@ -74,7 +74,15 @@ export async function getUploadFormData< const bb = busboy({ headers: request.headers, defParamCharset: 'utf8', limits }); const fields = Object.create(null) as K; - let uploadedFile: UploadResultWithOptionalFile | undefined; + let uploadedFile: UploadResultWithOptionalFile | undefined = { + fields, + encoding: undefined, + filename: undefined, + fieldname: undefined, + mimetype: undefined, + fileBuffer: undefined, + file: undefined, + }; let returnResult = (_value: UploadResultWithOptionalFile) => { // noop @@ -85,22 +93,13 @@ export async function getUploadFormData< function onField(fieldname: keyof K, value: K[keyof K]) { fields[fieldname] = value; - uploadedFile = { - fields, - encoding: undefined, - filename: undefined, - fieldname: undefined, - mimetype: undefined, - fileBuffer: undefined, - file: undefined, - }; } function onEnd() { if (!uploadedFile) { return returnError(new MeteorError('No file or fields were uploaded')); } - if (!('file' in uploadedFile) && !options.fileOptional) { + if (!options.fileOptional && !uploadedFile?.file) { return returnError(new MeteorError('No file uploaded')); } if (options.validate !== undefined && !options.validate(fields)) {