richardgirges / express-fileupload

Simple express file upload middleware that wraps around busboy
MIT License
1.52k stars 261 forks source link

abortOnLimit doesn't response 413 #263

Closed RayPS closed 10 months ago

RayPS commented 3 years ago

abortOnLimit: true stuck the request until client timeout instead of responding 413. Related to #84 but OP closed without solution.

richardgirges commented 3 years ago

I could use some help if you or anyone else is interested in submitting a PR to resolve this issue. Otherwise I will try to look into this over the weekend. Thanks for shedding light on #84!

richardgirges commented 3 years ago

Just released v1.2.1 - I believe there may have been some work done to alleviate this issue. Please let me know if that's the case.

RayPS commented 3 years ago

Hello, v1.2.1 doesn't fix the issue, I believe closeConnection() in processMultipart.js:86 is not working.

https://github.com/richardgirges/express-fileupload/blob/da968ef0365eba4bad73909737700798d89d2ad7/lib/processMultipart.js#L86

RomanBurunkov commented 10 months ago

I can confirm that for versions 1.4.1 and 1.4.2 abortOnLimit: true closes the connection. Basically logs with debug: true looks this way:

Express-file-upload: Size limit reached for inpFile->ubuntu-22.04.1-live-server-amd64.iso, bytes:5242880
Custom limitHandler called.
Express-file-upload: Aborting upload because of size limit inpFile->ubuntu-22.04.1-live-server-amd64.iso.
Express-file-upload: Headers already sent, can't close connection.
Express-file-upload: Cleaning up temporary file C:\Users\User\Desktop\dev\projects\express-fileupload-example\temp\tmp-1-1698861764468...
Express-file-upload: Cleaning up temporary file C:\Users\User\Desktop\dev\projects\express-fileupload-example\temp\tmp-1-1698861764468 done.

So I suppose this is fixed sometime ago. Otherwise please share more details and steps to reproduce.