improve Multer limits validation and error reporting, update related tests

Author: ShubhamOulkar

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)
   }
 

lib/multer-error.js

@@ -8,15 +8,19 @@ 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) {
   Error.captureStackTrace(this, this.constructor)
   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)

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() })