jaydenseric / graphql-upload

Middleware and a scalar Upload to add support for GraphQL multipart requests (file uploads via queries and mutations) to various Node.js GraphQL servers.
https://npm.im/graphql-upload
MIT License
1.43k stars 132 forks source link

graphqlUploadExpress maxFileSize restriction not work #274

Closed SergioSuarezDev closed 2 years ago

SergioSuarezDev commented 2 years ago

Hi guys

I have this config in my nestjs api main.ts

  const { API_UPLOADS_MAX_FILESIZE, API_UPLOADS_MAX_FILES } = process.env;
  app.use(
    graphqlUploadExpress({
      maxFieldSize: API_UPLOADS_MAX_FILESIZE,
      maxFileSize: API_UPLOADS_MAX_FILESIZE,
      maxFiles: API_UPLOADS_MAX_FILES
    })
  );

Config in .env file:

API_UPLOADS_MAX_FILESIZE=20000000
API_UPLOADS_MAX_FILES=10

20000000 bytes = 20MB

But when i try to upload 70MB file, the api doesn't show any error, it is as if this restriction did not work

jaydenseric commented 2 years ago

It's supposed to emit a relevant error on the file upload stream:

https://github.com/jaydenseric/graphql-upload/blob/17993a7f2a41de952cb1d99f2018d67503df9fe6/public/processRequest.js#L288-L295

The test we have for maxFileSize doesn't seem to test the error event emits for a large file that streams in multiple chunks, instead it tests the scenario where createReadStream() throws:

https://github.com/jaydenseric/graphql-upload/blob/b47970970cce60f46bd731abbd8255f5ad8ca8c9/test/public/processRequest.test.mjs#L539-L608

Ideally both scenarios would be tested. Once I've worked through some other graphql-upload updates I can improve these tests and hopefully verify it's working correctly.

jaydenseric commented 2 years ago

A common gotcha about the graphql-upload limit options: Make sure your Apache/nginx config is looser than the graphql-upload configured limits, that way errors will be handed nicely the GraphQL way instead of however your server deals with oversized requests, etc.

SergioSuarezDev commented 2 years ago

silly mistake hehe I had to make a parseint of the data that it loads from the .env because they are always strings, now it works

const { API_UPLOADS_MAX_FILESIZE, API_UPLOADS_MAX_FILES } = process.env; app.use( graphqlUploadExpress({ maxFieldSize: parseInt(API_UPLOADS_MAX_FILESIZE), maxFileSize: parseInt(API_UPLOADS_MAX_FILESIZE), maxFiles: parseInt(API_UPLOADS_MAX_FILES) }) );