Quickly closing connection after sending StreamableFile crashes server

[danielkappelle]

Is there an existing issue for this?

• I have searched the existing issues

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:

Warning: Binary output can mess up your terminal. Use "--output -" to tell
Warning: curl to output it to your terminal anyway, or consider "--output
Warning: <FILE>" to save to a file.

And the NestJS logs are

[Nest] 8844  - 08/12/2022, 10:35:26 AM   ERROR [ExpressAdapter] Premature close
Error: Premature close
    at new NodeError (node:internal/errors:372:5)
    at ServerResponse.onclose (node:internal/streams/end-of-stream:142:30)
    at ServerResponse.emit (node:events:539:35)
    at Socket.onServerResponseClose (node:_http_server:236:23)
    at Socket.emit (node:events:539:35)
    at TCP.<anonymous> (node:net:709:12)
Error: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:372:5)
    at ServerResponse.setHeader (node:_http_outgoing:576:11)
    at ServerResponse.header (/Users/danielkappelle/tmp/nest-stream-curl/test-project/node_modules/express/lib/response.js:794:10)
    at ServerResponse.send (/Users/danielkappelle/tmp/nest-stream-curl/test-project/node_modules/express/lib/response.js:174:12)
    at StreamableFile.handleError (/Users/danielkappelle/tmp/nest-stream-curl/test-project/node_modules/@nestjs/common/file-stream/streamable-file.js:14:17)
    at ReadStream.<anonymous> (/Users/danielkappelle/tmp/nest-stream-curl/test-project/node_modules/@nestjs/platform-express/adapters/express-adapter.js:43:22)
    at Object.onceWrapper (node:events:642:26)
    at ReadStream.emit (node:events:539:35)
    at emitErrorNT (node:internal/streams/destroy:157:8)
    at emitErrorCloseNT (node:internal/streams/destroy:122:3)

I think the issue might be related to #9819 and #9759

Minimum reproduction code

https://github.com/danielkappelle/nestjs-stream-curl

Steps to reproduce

1. Install packages npm i
2. Start server npm run start:dev
3. Try to download image using curl 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

• I don't know. Or some 3rd-party 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 (see below)

Other package

No response

NestJS version

9.0.9

Packages versions

 "dependencies": {
    "@nestjs/common": "9.0.9",
    "@nestjs/core": "9.0.9",
    "@nestjs/platform-express": "9.0.9",
    "reflect-metadata": "^0.1.13",
    "rimraf": "^3.0.2",
    "rxjs": "^7.2.0"
  },
  "devDependencies": {
    "@nestjs/cli": "^9.0.0",
    "@nestjs/schematics": "^9.0.0",
    "@nestjs/testing": "^9.0.0",
    "@types/express": "^4.17.13",
    "@types/jest": "28.1.4",
    "@types/node": "^16.0.0",
    "@types/supertest": "^2.0.11",
    "@typescript-eslint/eslint-plugin": "^5.0.0",
    "@typescript-eslint/parser": "^5.0.0",
    "eslint": "^8.0.1",
    "eslint-config-prettier": "^8.3.0",
    "eslint-plugin-prettier": "^4.0.0",
    "jest": "28.1.2",
    "prettier": "^2.3.2",
    "source-map-support": "^0.5.20",
    "supertest": "^6.1.3",
    "ts-jest": "28.0.5",
    "ts-loader": "^9.2.3",
    "ts-node": "^10.0.0",
    "tsconfig-paths": "4.0.0",
    "typescript": "^4.3.5"
  },

Node.js version

16.15.1

In which operating systems have you tested?

• macOS
• Windows
• Linux

Other

No response

[kamilmysliwiec]

cc @jmcdo29

[SirReiva]

@danielkappelle you need setErrorHandler to avoid app crash

[danielkappelle]

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

[micalevisk]

I believe that behavior should be the default. Let's wait for Jay

[jmcdo29]

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 if (!req.destroyed), but I'll have to figure out what the equivalent will be in fastify. This way, the client is in control of if the request stays open and gets a response or not, and we don't crash the server. I'll work on a PR for this today

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

[jmcdo29]

@danielkappelle you need setErrorHandler to avoid app crash

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

[kamilmysliwiec]

Let's track this here #10106