Skip to content
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

Incorrect behavior of per-request limits #496

Closed
2 tasks done
hash0000 opened this issue Nov 28, 2023 · 4 comments · May be fixed by johanbook/meet#855 or johanbook/meet#868
Closed
2 tasks done

Incorrect behavior of per-request limits #496

hash0000 opened this issue Nov 28, 2023 · 4 comments · May be fixed by johanbook/meet#855 or johanbook/meet#868

Comments

@hash0000
Copy link
Contributor

hash0000 commented Nov 28, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.24.3

Plugin version

8.0.0

Node.js version

21.2.0

Operating system

Windows

Operating system version (i.e. 20.04, 11.3, 10)

W11. 23H2 (22631.2715)

Description

Clear and concise:

  1. Inaccuracy in documentation (possibly);
  2. Inability to specify limits for each route separately.

I also want to clarify that I am using TypeScript.
TypeScript version: 5.3.2.

Steps to Reproduce

  1. From plugin types:
    image
    In BusboyConfig interface simply no throwFileSizeLimit.

  2. Set limits for request.files()
    image
    Set throwFileSizeLimit: true globally
    image

Send larger files and in larger quantities. As a result, we do NOT get exception.
However, If you specify the same parameters globally or for the saveRequestFiles function, the limits will work correctly.

Expected Behavior

  1. The first thing I would like to say is that the instructions in the documentation are incorrect (or am I wrong?).
    Example from docs:
    image
    However, I cannot specify such a parameter due to differences in types. files() allow BusboyConfig interface, in which there is no throwFileSizeLimit parameter. It is only available in the FastifyMultipartBaseOptions interface.
    image

  2. What I'm trying to do: set basic global limits when registering a plugin and specify the limits of each route separately, for part of them, as an overwrite of the global ones.
    Global:

 const fastifyMultipartOptions: FastifyMultipartOptions = {
    limits: {
      fieldNameSize: 100,
      fieldSize: 100,
      fields: 5,
      fileSize: 1,
      headerPairs: 2000,
      files: 5,
    },
    throwFileSizeLimit: true,
  };
  await app.register(fastifyMultipart, fastifyMultipartOptions);

For route (I can't set throwFileSizeLimit because of the problem described earlier):
await request.files({ limits: { files: 1, fileSize: 1 } });

The limits specified globally work correctly, but the specified limits for a request are completely ignored.
I tried this way: use saveRequestFiles(opts), where setting limits works correctly, however, I cannot get toBuffer() from the savedRequestFiles array, which makes this function not relevant for me.

@iamarthurk
Copy link

+1

@mcollina
Copy link
Member

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

(Alternatively a PR with a proposed change would also be fine).

@hash0000
Copy link
Contributor Author

hash0000 commented Dec 27, 2023

@mcollina, I think I found the problem.
I tested again and found a correlation with whether I call file.toBuffer() or not. If file.toBuffer() is not called after receiving a file stream, then the limits are ignored.
Is this how the limits work correctly?

@mcollina
Copy link
Member

The limits are enforced when the file is consumed (when the data is read from the underlying socket).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants