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

Fix out-of-band error event from busboy #1177

Open
wants to merge 1 commit into
base: lts
Choose a base branch
from

Conversation

max-mathieu
Copy link

Fixes #1176

@MelleB
Copy link

MelleB commented Apr 6, 2023

I'm experiencing this as well. Can someone review this?

Tests which actually trigger this behavior are missing from this PR.

@nerder
Copy link

nerder commented Mar 23, 2024

Any chance this will be merged any time soon?

@nandorcsupor-met
Copy link

Any chance this will be merged any time soon?

Yep we desparately need this. Only solution is to downgrade the package.

@podplatnikm
Copy link

Bump, any update?

@raymond-tyr
Copy link

Any update on this?
I've validated this approach during testing and it works

@CodeFoxLk
Copy link

any update?

@IamLizu IamLizu requested a review from a team September 14, 2024 12:04
Copy link
Member

@IamLizu IamLizu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the feature branch requires a rebase or it least the test commands needs to be updated to,

standard && mocha --reporter spec --bail --exit --check-leaks test/

I have noticed that more test fails if its just kept to standard && mocha.

And just ran the test after changing the test command, still 2 test fails. One of them looks like this,

  1) Disk Storage should process parser/form-data POST request:

      Uncaught AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

1803 !== 1778

      + expected - actual

      -1803
      +1778

      at test\disk-storage.js:45:14
      at test\_util.js:26:7
      at done (lib\make-middleware.js:47:7)
      at indicateDone (lib\make-middleware.js:51:68)
      at lib\make-middleware.js:157:11
      at WriteStream.<anonymous> (storage\disk.js:43:9)
      at WriteStream.emit (node:events:526:35)
      at finish (node:internal/streams/writable:937:10)
      at node:internal/streams/writable:918:13
      at process.processTicksAndRejections (node:internal/process/task_queues:82:21)

I am not posting the 2nd one because it says "operation not permitted", thats mostly because I am on personal PC, windows right now and I don't want to mess it up lol. I will run the test from a different PC tomorrow.

@max-mathieu
Copy link
Author

@IamLizu I branched off lts -- should I rebase to master? I ran this branch tests and they passed. I also merged master in and ran tests with standard && mocha --reporter spec --bail --exit --check-leaks test/ and they also all passed

Since this issue allows an attacker to craft a malformed request and take down any express app that uses multer, should that be a CVE? That's one reason why I branched off lts since no releases have been made since 1.4.5-lts.1

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

Successfully merging this pull request may close these issues.

8 participants