[QUESTION] Busboy and Expressjs - response not sent back to the client if next(new Error(...)) is called inside "file" event

[DvDream]

Hi, I am trying to load a zip file through busboy-connect. I am using expressjs for the server side. The flow would be to load the zip file, check if the content of the zip (which is a folder) already exists in the file system and if not, unzip it e save it in the desired path.

In the case that the file does not exist everything is fine, the problem I am having is with my custom middleware that handle the error for an already existing file. The problem is that in this case the request remain pending and it is not sent back to the client.

I also tried to use my custom middleware outside the busboy "file" event and in that case everything is fine (i.e. the error is thrown ) the response is sent back to the client.

const busboy = require('connect-busboy');
app.use(busboy({
  highWaterMark: 5 * 1024 * 1024, // Set 2MiB buffer
})); // Insert the busboy middle-ware

    app.post("/upload_report", async (req, res, next) => {
    let uploadPath = 'C:/Users/admin/Desktop/reports/allure-reports';
    let _filename;

    try{
    req.busboy.on('file', (fieldname, file, filename) => {

      if (req.complete) {
        return;
      }

      if (!fs.existsSync(path.join(uploadPath, `${filename.filename}`).replace(/.zip/, ''))) 
      {

        console.log(`Upload of '${filename.filename}' started`);
        _filename = `${filename.filename}`;

        // Create a write stream of the new file
        const fstream = fs.createWriteStream(path.join(uploadPath, `${filename.filename}.temp`));

        // This is here incase any errors occur
        fstream.on('error', function (err) {
          console.log(err);
        });

        // Pipe it trough
        file.pipe(fstream);

      } else {
        req.unpipe(req.busboy);
        next(new Error("Custom error to pass to middleware"));
        //next(new Error('This folder already exists in the filesystem. You may want to consider to contact the system administrator to delete->replace it.'));
      }

      // });
    });

    req.busboy.on("close", async () => {

      console.log("File caricato, si procede con l'estrazione del file...");

      console.log(`Upload of '${_filename}' finished`);
      await fse.rename(path.join(uploadPath, `${_filename}.temp`), path.join(uploadPath, `${_filename}`));

      try {

        await extract(path.join(uploadPath, `${_filename}`), { dir: uploadPath });
        await fse.remove(`${uploadPath}/${_filename}`);
        console.log(`Rimosso ${uploadPath}/${_filename}`);
        return res.status(200).send({ title: 'upload_report_success', message: `Report ${_filename} correctly uploaded` });

      } catch (e) {

        await fse.remove(`${uploadPath}/${_filename}`);
        console.log(`${e} - Rimosso ${uploadPath}/${_filename}`);
      }
    });

    req.pipe(req.busboy); // Pipe it trough busbsoy

       } catch (e) {
        return res.status(500).send({ title: 'upload_report_error', message: e.message });
       }

       });

this is my middleware (which is put after every other app.use() ones.

 app.use((err, req, res, next) => {
  console.log(err.message);

   res.status(500).json({ title: 'upload_report_error', message: 'This folder already exsists in the filesystem. You may want to consider to 
   contact the system administrator to delete->replace it.' });
    })

The problem is that I would need to know the name of the file server side but I can check it only inside the "file" event

[mscdex]

There are a few issues I see with the code you've posted:

1.   if (req.complete) {
       return;
     }

   is not correct, you should always consume file whether you're interested in it or not if you have a 'file' event handler. At the minimum that means using something like file.resume(); before your return.

2. Using the filename as-is like that puts you at risk for security issues if the filename is maliciously crafted (e.g. contains a relative path to escape your upload directory).

3. If the request is still pending after you've verified a response has been sent, then perhaps something (in Express or elsewhere) is still waiting for the request to be completely drained. If that's the case, you'll need to add something like req.resume() after you req.unpipe(busboy) to ensure any remaining data is drained out.