-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
Quickly closing connection after sending StreamableFile crashes server #10105
Comments
cc @jmcdo29 |
@danielkappelle you need setErrorHandler to avoid app crash |
@SirReiva Thank you very much, that indeed catches the error and prevents the app from crashing. Would it be an idea to add this to the docs over at https://docs.nestjs.com/techniques/streaming-files ? |
I believe that behavior should be the default. Let's wait for Jay |
Okay, so it looks like this is coming in because curl closes the stream as quickly as possible, meaning that this call cannot properly be made. We can do a check here EDIT: this works out of the box with fastify, it's just a problem for the handler we've implemented with express. Should be a quick and easy PR |
This prevents the crash for sure, but it also would mean that any error in the stream, including a not-found file, would result in a logged value and that's it. In the case of a not-found file there'd be no response, and the client would be left hanging, just a heads up |
In the case of using `curl` or a similar tool on the command line, or if the client decides to end the request for a streamed file early without the check we would end up causing a server crash. Now, we will just not send the response as the client has already decided how to move on. fix: nestjs#10105
Let's track this here #10106 |
Is there an existing issue for this?
Current behavior
When sending a larger file (testing with 10MB now) using
StreamableFile
and closing the connection very quickly after the connection was opened the NestJS server crashes. First a 'Premature close' error is thrown, but then Nest attempts to send a 400 status, which throws a 'Cannot set headers after they are sent to the client'. This error crashes the server completely.I found out because I tried to curl an image from the server, but curl warns for displaying binary output in the terminal. It then closes the connection very quickly leading to this behavior.
Of course this not a normal way to interact with the server, but it could be a potential security flaw as it makes it very easy to cause a DoS.
The curl output will be:
And the NestJS logs are
I think the issue might be related to #9819 and #9759
Minimum reproduction code
https://github.com/danielkappelle/nestjs-stream-curl
Steps to reproduce
npm i
npm run start:dev
curl http://localhost:3000
Expected behavior
The error should be caught, or maybe it should not attempt to send the 400 status.
In any case the server should not completely crash due to a simple curl request.
Package
@nestjs/common
@nestjs/core
@nestjs/microservices
@nestjs/platform-express
@nestjs/platform-fastify
@nestjs/platform-socket.io
@nestjs/platform-ws
@nestjs/testing
@nestjs/websockets
Other package
No response
NestJS version
9.0.9
Packages versions
Node.js version
16.15.1
In which operating systems have you tested?
Other
No response
The text was updated successfully, but these errors were encountered: