expressjs / multer

Node.js middleware for handling `multipart/form-data`.
MIT License
11.55k stars 1.05k forks source link

Multer does not throw an error when limits: fileSize is exceeded and hangs #602

Open thunderbird3 opened 6 years ago

thunderbird3 commented 6 years ago

I have the following code to handle profile pic uploads. Everything works fine if under the limits: fileSize, but when fileSize is exceeded no error is thrown. Error handling based on https://github.com/expressjs/multer/issues/336. I have a custom disk storage engine. The problem with this is that I cannot send any error message or a response to react frontend.

var storage = ProfileStorage({
  square: true,
  responsive: true,
  greyscale: false,
  quality: 60
});

var limits = {
  //files: 1, // allow only 1 file per request
  fileSize: 100 * 1024
};

var fileFilter = function(req, file, cb) {
  var allowedMimes = ['image/jpeg', 'image/pjpeg', 'image/png', 'image/gif'];

  if (_.includes(allowedMimes, file.mimetype)) {
    cb(null, true);
  } else {
    cb(new Error('Invalid file type. Only jpg, png and gif image files are allowed.'));
  }
};

var upload = multer({
  storage: storage,
  limits: limits,
  fileFilter: fileFilter
});

The post method:

var profileUpload = upload.single('Profiilikuva');

module.exports = app => {
  app.post('/api/profile/save', requireLogin, (req, res) => {
    console.log('Entered profile save');

    profileUpload(req, res, (err) => {
      console.log('Entered profile upload.');
      if (err) {
        console.log('Error from multer: ', err);
        return res.status(400).send(err);
      }

      if (req.file) {
        console.log('Stored file: ', req.file.filename); // Storage engine stores under random filename
      }

      const {firstName, lastName, email, address,
        postalCode, descriptionText, lat, lng } = req.body;

      req.user.profileCompleted = true;
      req.user.firstName = firstName;
      // some more of these...
      req.user.save().then((user) => { 
        console.log('User saved');
        return res.send(user);
      }).catch(err => {
        console.log('Error saving user. ', err);
        return res.status(400).send('Error while saving user information');
      });
    });
  });
};

Log output when uploading 75KB:

Entered profile save
_handleFile called. { fieldname: 'Profiilikuva',
   originalname: '75KB.jpg',
   encoding: '7bit',
   mimetype: 'image/jpeg' }
 Entered profile upload.
 Stored file:  0b3dc28730ecc387115a03b3f860af20_lg.jpg
 User saved

Log output when uploading 190KB:

 Entered profile save
 _handleFile called. { fieldname: 'Profiilikuva',
   originalname: '190KB.jpg',
   encoding: '7bit',
   mimetype: 'image/jpeg' }
_removeFile called
beac0n commented 6 years ago

I have exactly the same issue. I completely removed this option and handle files which are too big on my own

thunderbird3 commented 6 years ago

yup, I handle the file size on the client as well. Though I think it is my custom storage engine that messes things up. Based on other issues i read the default disk storage engine may be able to handle file limits at least for one file. I have not had a chance to look at this in any detail. Also may be related to #447 . Seems the error handling is a bit tricky.

beac0n commented 6 years ago

"Though I think it is my custom storage engine that messes things up" I am using the google cloud storage engine for buckets.

"Seems the error handling is a bit tricky." That's a huge understatement.

I think the issue might have something to do with the file streams. As far as I know, the file streams for writing to the file system have more meta information than, e.g. streams to google storage buckets.

sainisagar310 commented 5 years ago

I am also facing the same issue. It doesn't throw an error when the file limit exceeded.

Nziranziza commented 4 years ago

I have not defined fileSize limits but Multer does not throw an error but keeps hanging when uploading larger files( like > 1MB).

jonchurch commented 4 years ago

I'm not sure if this is a bug but I'm labeling and self assigning to look at it another time

jonchurch commented 4 years ago

I was unable to reproduce this issue. Here is a codesandbox where I've attempted reproduction.

When fileSize is set, and a file is uploaded which is larger than that limit, I was able to receive an error.

Leaving this open for now to wait for more info, if anyone is experiencing this please provide a reproduction case if you're able to! ❤️

theartofnonso commented 4 years ago

I am currently using the latest version 1.4.2 and I have the following limit set: const upload = multer({ dest: "avatars", limit: { fileSize: 1000000, }, });

It still doesn't throw any error

jonchurch commented 4 years ago

@nonsobiose Can you provide a minimal reproduction case? I haven't been able to reproduce this so far and will need to see your code to try and debug.

theartofnonso commented 4 years ago

@jonchurch

const upload = multer({
    limit: {
        fileSize: 1000000,
    },
    fileFilter(req, file, cb) {
        if (file.originalname.match(/\.(jpg|jpeg|png)\b/)) {
            return cb(undefined, true)
        }
        cb(new Error("File must be an image!"));
    }
}).single("avatar");
jonchurch commented 4 years ago

@nonsobiose I would need to see a standalone example of your code where you're experiencing the issue to be able to figure out what's happening, but see my earlier comment with the link to a codesandbox where I'm handling the error passed to the upload callback triggered by the fileSize limit.

When uploading the file with your upload function, is the error object not being passed to the callback provided to upload?

prashand commented 4 years ago

This is so weird. It works perfectly on your sandbox @jonchurch . I'm here because I tried to get the file filter errors working:

export const uploadImage = multer({
    limits:{
        fileSize: 5241288,
        files: 1
    },
    fileFilter:  (req, file, callback) => {
        callback(new Error('Bad file type'), false)
    },
})
export const setAvatarImage = (req: Request, res: Response, next: NextFunction) => {   
    uploadImage.single('image')(req, res, error => {
        if(error) return res.status(400).send({errors})
    }
}

Which returns

{
    "error": {
        "storageErrors": []
    }
}

But the sandbox that I made seems to be working perfectly https://codesandbox.io/s/vibrant-lovelace-3bn90

Edit: I ended up manually setting req.fileValidationError on the request object because the documented way of cb(new Error('Text'), false) was not working for me.

Inspired by this answer: https://stackoverflow.com/a/35069987/1800515

ptejas26 commented 4 years ago

The problem still persists, did anyone got solution of this problem ?

icoleto commented 4 years ago

According to documentation here: https://github.com/expressjs/multer#error-handling I tried to follow the error handling this way:

if (err instanceof multer.MulterError) {
      // A Multer error occurred when uploading.
}

But the error is not an instanceOf MulterError but HttpError, and follows the next structure:

{
    message: {
        error: "Payload Too Large",
        message: "File too large",
        statusCode: 413
    },
    response: {
        error: "Payload Too Large",
        message: "File too large",
        statusCode: 413
    },
    stack: "...",
    status: 413,
    __proto__: ...
}

I used this (simplified) workaround using NestJS:

import {HttpException } from '@nestjs/common'; 
if (err instanceof HttpException) {
     if (err.getStatus() === 413) {
          // response.status(statusCode).json(response);
     }
cq-ubaid-khan commented 4 years ago

This is so weird. It works perfectly on your sandbox @jonchurch . I'm here because I tried to get the file filter errors working:

export const uploadImage = multer({
    limits:{
        fileSize: 5241288,
        files: 1
    },
    fileFilter:  (req, file, callback) => {
        callback(new Error('Bad file type'), false)
    },
})
export const setAvatarImage = (req: Request, res: Response, next: NextFunction) => {   
    uploadImage.single('image')(req, res, error => {
        if(error) return res.status(400).send({errors})
    }
}

Which returns

{
    "error": {
        "storageErrors": []
    }
}

But the sandbox that I made seems to be working perfectly https://codesandbox.io/s/vibrant-lovelace-3bn90

Edit: I ended up manually setting req.fileValidationError on the request object because the documented way of cb(new Error('Text'), false) was not working for me.

Inspired by this answer: https://stackoverflow.com/a/35069987/1800515

remove this: callback(new Error('Bad file type'), false)

to this: callback('Bad file type', false)

kodwi commented 4 years ago

I faced this error using multer in nest.js app. It seems like multer just ignores limits.fileSize and goes on handling the uploaded file calling destination, filename callbacks etc, without any error. I just use diskStorage, not custom one.

Multer's version is 1.4.2.

bhavinpatel31 commented 3 years ago

Same issue here using multerGoogleStorage.

ThalesMatoso commented 3 years ago

@jonchurch Same issue here using multerGoogleStorage

ThalesMatoso commented 3 years ago

Improvised solution

image

Majd-eddine-BEN-TAHAR commented 3 years ago

this is a solution worked for me , first i configured multer : const upload = multer({ storage: storage, fileFilter: fileFilter, limits: { fileSize: 1024 * 1024 }, });

then in my routes i use it like this : router.post( "/image", upload.single("image")); ` then in express error middleware i checked for the type of the error first :

module.exports = (error, req, res, next) => { if (error instanceof multer.MulterError) { error.status = 413; error.message = "image too large, max size is 1mb!"; } const status = error.status || 500; const message = error.message; const response = { status: status, error: message }; res.status(status).json(response); };

miestasmia commented 3 years ago

This is causing issues on production for me; while it does in fact throw an error when a file is too big, it does not appear to check the file size by comparing each chunk in the stream, but rather it waits until it has received the entire file and then throws an error.

I tried curling /dev/zero straight into my server and it made everything hang. I have not been able to make any of the suggestions in this thread work.

I am using:

multer({
  dest: os.tmpdir(),
  limits: {
    fileSize: 1024**2
  }
})

but it's still happy to let me throw gigabytes of garbage at it only to calculate the total file size at the very end.

miestasmia commented 3 years ago

I may have determined the (partial) cause of this. The busboy event limit is correctly listened for and passed to abortWithCode which passes it to abortWithError in lib/make-middleware.js, however abortWithError then waits for pendingWrites.onceZero which never finishes, see https://github.com/expressjs/multer/blob/master/lib/make-middleware.js#L64. I will try track this down in Counter to determine why that is the case.

ekatrand commented 3 years ago

Hi,

I was having issues with this. When I put the limits before the storage (and outside the storage part) it started working for me. Not exactly the same, just hoping to help someone.

const maxSize = 1 1000 1000; const uploader = multer({ limits: { fileSize: maxSize }, //Putting it here works, before and outside the storage storage: multerS3({ s3: s3, bucket: "shopitemimages", acl: "public-read", limits: { fileSize: maxSize }, //Putting it here seem to give error when its already to late - meaning its useless for my purpose contentType: multerS3.AUTO_CONTENT_TYPE, metadata: function (req, file, cb) { cb(null, { fieldName: "lets see what we want as fieldvalue" }); }, key: function (req, file, cb) { cb(null, Date.now().toString()); }, }), });

jayeshchoudhary commented 2 years ago

Improvised solution

image

it worked for me Thank you so much for this :)

wajeehassan123 commented 2 years ago

I have not defined fileSize limits but Multer does not throw an error but keeps hanging when uploading larger files( like > 1MB).

I am having same problem right now. Did you fix your problem?

ritanshu77 commented 2 years ago

i got error from this ...limits: {fileSize: 500}

iners-max commented 2 years ago

Still suffering from this in 2022... can someone help?

Mahitocode commented 2 years ago

Me too facing the issue can some one please help ?

dsmh commented 2 years ago

2022 and the error sitlls, Usings NestJS I use this, but is useless:

@Patch() @UseInterceptors( FileInterceptor('foto', { storage: diskStorage({ destination: STORAGE.PORTEROS.FOTOS, filename: editarNombreArchivo, }), limits: { fileSize: 2097152, } }), )

bambootv commented 1 year ago

I can't limit less than 1MB. Ex: fileSize: 0,5 1024 1024 Multer still upload and don't throw an error.

A solution is use fieldSize instead fileSizebut I think that isn't best practice.

alstn113 commented 1 year ago

same too, how to solve custom limit error?

hananbeer commented 1 year ago

in case anyone else tried to use string like me, e.g. '4mb', it does not work. use integers, 102410244

roziputra commented 1 year ago

i have same issue

hananbeer commented 1 year ago

i have same issue

did you try using integer gor filesize as I suggested above? worked for me

roziputra commented 1 year ago

i have same issue

did you try using integer gor filesize as I suggested above? worked for me i'm using integer coz it only accept number MulterModule.register({ storage: diskStorage({ destination: './data/ttb', filename(req, file, callback) { callback(null, ${Date.now()}-${file.originalname}); }, }), limits: { files: 6, fieldSize: 100, }, fileFilter: (req, file, cb) => { console.log('##', file.size); cb(null, false); }, }),

hananbeer commented 1 year ago

I believe it is fileSize you wrote fieldSize

Nabil-Nasr commented 1 year ago

I have not defined fileSize limits but Multer does not throw an error but keeps hanging when uploading larger files( like > 1MB).

me too but bigger than your size

AlonMiz commented 1 year ago

I've added an additional middleware to handle file limits middleware


export const processFileMiddleware = multer({
  storage: memoryStorage(),
}).single('file');

export const fileLimitsMiddleware = asyncHandler(
  (request: express.Request, response: express.Response, next: express.NextFunction) => {
    if (!request.file) {
      throw new ServerError({
        code: ErrorCodes.FILE_NOT_FOUND,
        message: 'File not found',
        statusCode: 400,
      });
    }
    if (request.file.size > config.fileStorage.maxFileSize) {
      throw new ServerError({
        code: ErrorCodes.FILE_TOO_LARGE,
        message: `File too large. max ${humanFileSize(
          config.fileStorage.maxFileSize
        )}, received ${humanFileSize(request.file.size)}`,
        statusCode: 400,
      });
    }
    next();
  }
);

later I've used

protectedRouter.post('/file/upload', processFileMiddleware, fileLimitsMiddleware, uploadFileController);

working tests with vitest and supertest. make sure to have valid file paths

import supertest from 'supertest';
import { afterAll, beforeAll, describe, expect, test } from 'vitest';
import { Express } from 'express';
import { Server } from 'http';
import path from 'path';
import { initServer } from '../app';

describe('Upload file', () => {
  let app: Express;
  let server: Server;
  let request: supertest.SuperTest<supertest.Test>;
  beforeAll(async () => {
    ({ server, app } = await initServer());
    request = supertest(app);
  });

  afterAll(() => {
    server.close();
  });

  describe('POST /api/file/upload', () => {
    const bigFile = path.join(__dirname, './mocks/big.pdf');
    const smallFile = path.join(__dirname, './mocks/small.pdf');
    test('should upload successfuly', async () => {
      const res = await request
        .post('/api/file/upload')
        .set({ auth: 'any-token' })
        .attach('file', smallFile);

      expect(res.body).toEqual({ message: 'File uploaded successfully' });
      expect(res.status).toBe(200);
    });

    test('should return 400 if file is too large', async () => {
      const res = await request
        .post('/api/file/upload')
        .set({ auth: 'any-token' })
        .attach('file', bigFile);
      expect(res.status).toBe(400);
      expect(res.body).toEqual({
        errorCode: 'FILE_TOO_LARGE',
        message: 'File too large. max 1 byte, received 132 byte',
        statusCode: 400,
      });
    });
  });
});
drattansingh commented 1 year ago
const multer = require('multer')
const{v4:uuidv4}=require('uuid')

const fileStorage = multer.diskStorage({
  destination:(req, file, cb)=>{ cb(null, 'images') },
  filename: function(req, file, cb){ cb(null, uuidv4()) }
});

const fileFilter = (req, file, cb) => {
  const fileSize = parseInt(req.headers["content-length"]) // get the filesize
  if(file.mimetype.substring(0,5)!=='image') cb(new Error('Error - only image files allowed'), false)
  else if(fileSize>1024*1000) cb(new Error('Error - images must be under 1 Mb'), false)
  else cb(null, true) // no errors, then add the file
}

app.use(multer({ storage: fileStorage, fileFilter: fileFilter }).single('coverImage') )
dawidpstrak commented 1 year ago

I ended up handling file size limit by myself

exports.fileUpload = (req, res, next) =>
  multer().single("file")(req, res, () => {
    if (req.file.size > maxFileSizeInBytes) {
      return res.status(400).send("File upload size limit exceeded");
    }

    return next();
  });