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

express-fileupload seems to not respect cors with abortOnLimit #303

Open
andrecp opened this issue Feb 7, 2022 · 2 comments
Open

express-fileupload seems to not respect cors with abortOnLimit #303

andrecp opened this issue Feb 7, 2022 · 2 comments

Comments

@andrecp
Copy link

andrecp commented Feb 7, 2022

Hey folks,

I was playing with express-fileupload today and hit what I think is a bug.

const express = require('express');
const cors = require('cors');
const fileUpload = require('express-fileupload');

const app = express();
app.use(cors());
app.use(express.json());
app.use(fileUpload({
  useTempFiles: true,
  tempFileDir: '/tmp/',
  limits: { fileSize: 50 * 1024 * 1024 },
  abortOnLimit: true,
  preserveExtension: true,
}));

With the above, if my upload view is over the limit (50MB), it should return a 413, however, I believe because it isn't setting the CORS headers correctly it doesn't return 413, instead the client side gets a CORS error.

This is my view

app.post('/upload', (req, res) => {
    const uploadedFile = req.files[Object.keys(req.files)[0]];
    uploadedFile.mv(`/tmp/user1/${uploadedFile.name}`, (err) => {
      if (err) { return res.status(500).send(err); }
      return res.send('File uploaded!');
    });
});

If I register my own limitHandler it then works correctly and the cors headers get set

app.use(fileUpload({
  useTempFiles: true,
  tempFileDir: '/tmp/',
  limits: { fileSize: 50 * 1024 * 1024 },
  abortOnLimit: true,
  // eslint-disable-next-line no-unused-vars
  limitHandler: (req, res, next) => {
    res.status(400).json({ error: 'File is too large.' });
  },
  preserveExtension: true,
}));

Another interesting thing is that with my limitHandler I need to check if the headers were already sent, otherwise my app crashes

  if (!res.headersSent) {
    const uploadedFile = req.files[Object.keys(req.files)[0]];
    uploadedFile.mv(`/tmp/user1/${uploadedFile.name}`, (err) => {
      if (err) { return res.status(500).send(err); }
      return res.send('File uploaded!');
    });
  }

Thank you

@richardgirges
Copy link
Owner

It looks like the headers being sent is a bug. I'm not sure what's going on with the CORS issue though, curious if the headers being sent by express-fileupload is creating a CORS issue as a red herring. Would you be able to provide a simple app with a repro?

@RomanBurunkov
Copy link
Collaborator

RomanBurunkov commented Nov 1, 2023

Hi,

Issue with headers were already sent fixed in version 1.4.2 by PR #238.
In the case described above, middleware won't call next and route handler will not be invoked.

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

3 participants