busboy icon indicating copy to clipboard operation
busboy copied to clipboard

`partsLimit` is being thrown when busboy is configured for one file

Open bahtou opened this issue 4 years ago • 4 comments

Busboy is configured as:

const options = {
    headers,
    defCharset: 'utf8',
    limits: {
      fieldNameSize: 0,
      fieldSize: 0,
      fields: 0,
      fileSize: 10496000,
      files: 1,
      parts: 1,
      headerPairs: 2000
    }
  };

A single file upload like so:

<body style="background-color: black">
  <form enctype="multipart/form-data" method="post">
    <label>single file
      <input type="file" name="file_v1" />
    </label><br />

    <button type="submit">Upload</button>
 </form>
</body>

Why would the partsLimit be thrown? Is there something else inside the multipart? On the other hand when I configure the options to limit the files to 2, and when I upload 2 files, the filesLimit is not thrown. The same for field limit. I would expect the same behaviour for partsLimit -- to be thrown when I have violated the limit, not fulfilled the limit.

bahtou avatar Feb 27 '22 18:02 bahtou

Same.

Minimal code to reproduce the issue here: https://github.com/reviewingcode/busboy-multipart-file-upload

^basically the example code from README - with limits and events for partsLimit, filesLimit, fieldsLimit.

https://github.com/expressjs/multer/issues/1105

mateeni-dev avatar Jun 29 '22 22:06 mateeni-dev

It seems that the issue has something to do with the part limit not being followed, and was introduced in this commit!

After looking at the code, I could not explain this line and I thought that this might be the cause of the issue: https://github.com/mscdex/busboy/blob/master/lib/types/multipart.js#L445

@mscdex Why do we break if the limit is reached or surpassed? Don't we need to break only if the limit is surpassed?

TheodosiouTh avatar May 02 '23 09:05 TheodosiouTh

It seems that the issue has something to do with the part limit not being followed, and was introduced in this commit!

After looking at the code, I could not explain this line and I thought that this might be the cause of the issue: https://github.com/mscdex/busboy/blob/master/lib/types/multipart.js#L445

@mscdex Why do we break if the limit is reached or surpassed? Don't we need to break only if the limit is surpassed?

When you look at the code changes, it looks like two changes collided. The event 'partsLimit' used to be emitted too late, now it is emitted when the limit is reached, as described in the readme file. But on the other hand, part limit, which was enforced in the same code chunk, was right. The check on line 445 seems indeed to be too zealous, and the new code drops parts too early.

I am by no means qualified to work on this code, just comparing versions without an understanding of the full context. But is this code still maintained (sorry, can't help on this :-( )? Thanks!

gom59 avatar Aug 16 '23 13:08 gom59

@mscdex hi, thanks for maintaining this lib so far

would you consider looking into this issue in the near future? or if not, could you please mention if this is a wont-fix type situation?

mateeni-dev avatar Oct 12 '23 00:10 mateeni-dev