From e42098cb3fa4b265f2f10ebe32ddccdc6828d619 Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Wed, 11 Jun 2025 11:32:56 +0530 Subject: [PATCH 1/2] improve Multer limits validation and error reporting, update related tests --- index.js | 22 +++++++++++++++-- lib/multer-error.js | 8 +++++-- test/error-handling.js | 54 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index d5b67eba..c8a0b4bb 100644 --- a/index.js +++ b/index.js @@ -4,6 +4,24 @@ var diskStorage = require('./storage/disk') var memoryStorage = require('./storage/memory') var MulterError = require('./lib/multer-error') +function checkLimitValue (limitKey, value) { + if (!Number.isInteger(value) || value < 0) { + throw new MulterError('LIMIT_OPTION_ERROR', `option "${limitKey}" value: ${value}`) + } + return value +} + +function getLimits (limitObj) { + if (!limitObj || typeof limitObj !== 'object' || Array.isArray(limitObj)) { + throw new TypeError('Expected limits to be a plain object') + } + var limits = {} + Object.keys(limitObj).forEach(key => { + limits[key] = checkLimitValue(key, limitObj[key]) + }) + return limits +} + function allowAll (req, file, cb) { cb(null, true) } @@ -17,7 +35,7 @@ function Multer (options) { this.storage = memoryStorage() } - this.limits = options.limits + this.limits = options.limits ? getLimits(options.limits) : options.limits this.preservePath = options.preservePath this.fileFilter = options.fileFilter || allowAll } @@ -91,7 +109,7 @@ function multer (options) { return new Multer({}) } - if (typeof options === 'object' && options !== null) { + if (typeof options === 'object' && options !== null && !Array.isArray(options)) { return new Multer(options) } diff --git a/lib/multer-error.js b/lib/multer-error.js index d56b00e8..d52ab175 100644 --- a/lib/multer-error.js +++ b/lib/multer-error.js @@ -8,7 +8,8 @@ var errorMessages = { LIMIT_FIELD_VALUE: 'Field value too long', LIMIT_FIELD_COUNT: 'Too many fields', LIMIT_UNEXPECTED_FILE: 'Unexpected field', - MISSING_FIELD_NAME: 'Field name missing' + MISSING_FIELD_NAME: 'Field name missing', + LIMIT_OPTION_ERROR: 'Limit option must be an integer' } function MulterError (code, field) { @@ -16,7 +17,10 @@ function MulterError (code, field) { this.name = this.constructor.name this.message = errorMessages[code] this.code = code - if (field) this.field = field + if (field) { + this.field = field + if (code === 'LIMIT_OPTION_ERROR') this.message += `: ${field}` + } } util.inherits(MulterError, Error) diff --git a/test/error-handling.js b/test/error-handling.js index c2b1f973..453a91e7 100644 --- a/test/error-handling.js +++ b/test/error-handling.js @@ -14,6 +14,60 @@ function withLimits (limits, fields) { } describe('Error Handling', function () { + const invalidPlainObj = ['not an plain object', ['storage', 'limits']] + + const invalidLimitOptions = [ + { option: 'fieldNameSize', value: -100 }, + { option: 'fieldSize', value: 0.5 }, + { option: 'fields', value: '10' }, + { option: 'files', value: 'foo' }, + { option: 'fileSize', value: 1048.15 }, + { option: 'parts', value: '0' }, + { option: 'headersPairs', value: -10.55 } + ] + + invalidLimitOptions.forEach(({ option, value }) => { + it(`should throw an invalid limit option error for ${option} with value ${value}`, () => { + assert.throws( + () => multer({ limits: { [option]: value } }), + (err) => { + return ( + err instanceof multer.MulterError && + err.name === 'MulterError' && + err.code === 'LIMIT_OPTION_ERROR' && + err.field === `option "${option}" value: ${value}` && + err.message === `Limit option must be an integer: option "${option}" value: ${value}` + ) + }, + `Expected multer to reject ${option} value for ${value}` + ) + }) + }) + + it('should throw argument type error if limits is not a plain object', function () { + invalidPlainObj.forEach(option=>{ + assert.throws( + () => multer({ limits: option }), + (err) => { + return err instanceof TypeError && err.message === 'Expected limits to be a plain object' + }, + 'Expected multer to throw TypeError for non-object limits' + ) + }) + }) + + it('should throw type error if options is not a plain object', function () { + invalidPlainObj.forEach(option=>{ + assert.throws( + () => multer(option), + (err) => { + return err instanceof TypeError && err.message === 'Expected object for argument options' + }, + 'Expected multer to throw TypeError for non plain object options' + ) + }) + }) + it('should be an instance of both `Error` and `MulterError` classes in case of the Multer\'s error', function (done) { var form = new FormData() var storage = multer.diskStorage({ destination: os.tmpdir() }) From f486b10dfcda94872bb06bafbdae11f8c7766ddd Mon Sep 17 00:00:00 2001 From: Shubham Oulkar Date: Wed, 11 Jun 2025 12:15:08 +0530 Subject: [PATCH 2/2] lint fixes --- test/error-handling.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/error-handling.js b/test/error-handling.js index 453a91e7..2dd43195 100644 --- a/test/error-handling.js +++ b/test/error-handling.js @@ -45,7 +45,7 @@ describe('Error Handling', function () { }) it('should throw argument type error if limits is not a plain object', function () { - invalidPlainObj.forEach(option=>{ + invalidPlainObj.forEach(option => { assert.throws( () => multer({ limits: option }), (err) => { @@ -57,7 +57,7 @@ describe('Error Handling', function () { }) it('should throw type error if options is not a plain object', function () { - invalidPlainObj.forEach(option=>{ + invalidPlainObj.forEach(option => { assert.throws( () => multer(option), (err) => {