jaydenseric / graphql-upload

Middleware and an Upload scalar 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 131 forks source link

Per request middleware options #294

Closed Narretz closed 2 years ago

Narretz commented 2 years ago

Currently, maxFileSize is a fixed value. However, the graphql server might have different upload operations that have different size allowances. While it's certainly possible to check the stream data length in a resolver, it would be nice if the maxFileSize option could be a function that receives the operation name and variables and and returns a maxFileSize. This should work because the operation is parsed before the multipart data is read.

What do you think about this?

jaydenseric commented 2 years ago

What would be more powerful would be to be able to customize any of the processRequest options based on the request context, not just maxFileSize. For example, you could have greater limits for authenticated users; if your authentication middleware ran before the graphql-upload middleware and the authentication status is available in the middleware context.

For graphql-api-koa execute middleware options I came up with a concept of an override function that receives the middleware context and returns options that override the static options.

Generating schemas and things can be performance expensive to do every request, so in the graphql-api-koa situation a way to define some things statically at server start and others per request made sense. For graphql-upload middleware the options are not very expensive to create per request, so we probably don't need a very clever solution.

The graphql-upload middleware options could be defined as a function that receives the middleware context and returns the options object. I'll have a think if we allow static options objects to be used as well, like the current API, or if we just force everyone to use a function.

Anyway, I probably won't get to this enhancement for a while due to higher priorities. You're not stuck; you can just make your own middleware that uses processRequest and generates the maxFileSize option per request. You can reference the current middleware:

The processRequest function is exported precisely so people can make their own middleware in situations like yours.

Narretz commented 2 years ago

Hi @jaydenseric, thanks for the thorough explanation. Per request middleware function would be great.

Regarding a custom middleware. As far as I can see it, you can't "just" wrap processRequest and pass per-request options, because it's processRequest that parses the graphql operations in the first place. So you'd need to parse the request body before calling processRequest.

Come to think of it, the first thing processRequest does is to create an instance of Busboy, passing in the options. Now in my mind I thought you could first parse the gql operation, and then apply the fileSize, but that doesn't work because you can't change the busboy options later.

So I guess the only option is to either parse the operations before, or check fileSize in the resolver.

Happy to be incorrect about this though 😉

NelsonFrancisco commented 2 years ago

@Narretz it seems you are correct.

Tried to create a custom processRequest method but yeah... it's the original processRequest that parses graphQL data.

So, in order to have custom options per request, and the mentioned maxFileSize, you'd have to

  1. call original processRequest with a virtual unlimited maxFileSize
  2. change maxFileSize according to the request. Up to you to chose the size.
  3. call processRequest again with the new maxFileSize

Definitely not cool

jaydenseric commented 2 years ago

Here is where busboy options are used:

https://github.com/jaydenseric/graphql-upload/blob/5d55705bead7e5518ddc86d2052fe2598c48a35f/processRequest.mjs#L58-L67

busboy actually takes the request headers, so we could have an API where per request you can receive the request context including the request headers and return busboy related option overrides (e.g. for maxFileSize). That would be useful for the use case already mentioned of greater limits for authenticated users; when authentication happens using request headers and the status of the authentication is available in the request context. But yes, that would not help you in regards to wanting to set options based upon details of the GraphQL operation that haven't streamed in yet. Some people (particularly Apollo Server users) put the GraphQL operation name in a request header for security or other reasons, so in theory that might help some people who could use that information to override certain limits.

If you want to customise file size limits and things within specific resolvers, then that has to happen as you are processing the file upload stream. You can meter the stream and emit errors and things or handle going over conditional limits however you like. I'm not going to produce code examples, but such things have been discussed in issues (either this repo, or one of the related ones I can't remember) before and people have shared snippets.

Come to think of it, people could actually have per request limits using the current API by wrapping the graphql-upload middleware? Untested example:

/**
 * Koa middleware for processing GraphQL multipart requests, with larger limits
 * for authenticated requests.
 * @type {import("koa").Middleware}
 */
async function graphqlUploadKoaPerRequest(ctx, next) {
  await graphqlUploadKoa(
    "user" in ctx.state
      ? {
          maxFileSize: 20000000,
          maxFiles: 20,
        }
      : {
          maxFileSize: 10000000,
          maxFiles: 10,
        }
  )(ctx, next);
}

Closing unless someone can point out a reason why these suggested approaches wouldn't work.