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.

Sign up for GitHub

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

Jump to bottom

partsLimit is being thrown when busboy is configured for one file #287

Open
bahtou opened this issue Feb 27, 2022 · 4 comments
Open

partsLimit is being thrown when busboy is configured for one file #287

bahtou opened this issue Feb 27, 2022 · 4 comments

Comments

@bahtou
Copy link

bahtou commented Feb 27, 2022 •
edited
Loading

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.
@mateeni-dev
Copy link

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.

expressjs/multer#1105

@gom59 gom59 mentioned this issue Nov 2, 2022
Open
@TheodosiouTh
Copy link

TheodosiouTh commented May 2, 2023 •
edited
Loading

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?

@gom59
Copy link

gom59 commented Aug 16, 2023

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!

@mateeni-dev
Copy link

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@bahtou @TheodosiouTh @mateeni-dev @gom59