Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
thunderbird3 opened this issue Jun 15, 2018 · 40 comments
Assignees

Comments

@thunderbird3
Copy link

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 #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
Copy link

beac0n commented Jul 28, 2018

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

@thunderbird3
Copy link
Author

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
Copy link

beac0n commented Jul 28, 2018

"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
Copy link

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

@Nziranziza
Copy link

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

@jonchurch jonchurch self-assigned this Jan 18, 2020
@jonchurch
Copy link
Member

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

@jonchurch jonchurch added bug and removed bug labels Jan 18, 2020
@jonchurch
Copy link
Member

jonchurch commented Jan 19, 2020

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
Copy link

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
Copy link
Member

@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
Copy link

theartofnonso commented Jan 28, 2020

@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
Copy link
Member

jonchurch commented Jan 29, 2020

@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
Copy link

prashand commented Feb 8, 2020

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
Copy link

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

@icoleto
Copy link

icoleto commented Jun 3, 2020

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
Copy link

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
Copy link

kodwi commented Jul 15, 2020

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
Copy link

Same issue here using multerGoogleStorage.

@ThalesMatoso
Copy link

@jonchurch Same issue here using multerGoogleStorage

@ThalesMatoso
Copy link

Improvised solution

image

@Majd-eddine-BEN-TAHAR
Copy link

Majd-eddine-BEN-TAHAR commented Mar 8, 2021

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
Copy link

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
Copy link

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
Copy link

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
Copy link

Improvised solution

image

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

@wajeehassan123
Copy link

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
Copy link

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

@iners-max
Copy link

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

@Mahitocode
Copy link

Me too facing the issue can some one please help ?

@dsmh
Copy link

dsmh commented Aug 18, 2022

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

@HoanKy
Copy link

HoanKy commented Dec 28, 2022

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
Copy link

same too, how to solve custom limit error?

@hananbeer
Copy link

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

@roziputra
Copy link

i have same issue

@hananbeer
Copy link

i have same issue

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

@roziputra
Copy link

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
Copy link

I believe it is fileSize you wrote fieldSize

@Nabil-Nasr
Copy link

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
Copy link

AlonMiz commented May 21, 2023

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
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests