-
-
Notifications
You must be signed in to change notification settings - Fork 112
feat: add per route busboy configuration #580
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some unit test?
Thank you! Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs are missing
|
||
interface FastifyContextConfig { | ||
multipartOptions?: Omit<BusboyConfig, 'headers'> | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a test for this type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
config: { | ||
multipartOptions: { | ||
limits: { | ||
fileSize: 1_000_000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file size limitation error isn’t caused by this configuration.
In this test, it should occur only when the request configuration is set, as described in the title.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@is2ei
Hello! The error is caused by this exact configuration; I've verified it locally. This configuration is also specified in the title, except for the multipart_options(multipartOptions) field name. If I've misunderstood something, please correct me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you share how you verified it locally?
I commented out the config, but the test still passed.
It seems the error may not be caused by the config.
diff --git a/test/multipart-fileLimit.test.js b/test/multipart-fileLimit.test.js
index 6aed6a5..d6d4462 100644
--- a/test/multipart-fileLimit.test.js
+++ b/test/multipart-fileLimit.test.js
@@ -331,13 +331,13 @@ test('should throw fileSize limitation error when used alongside attachFieldsToB
crypto.randomFillSync(randomFileBuffer)
fastify.post('/', {
- config: {
- multipartOptions: {
- limits: {
- fileSize: 1_000_000
- }
- }
- }
+ // config: {
+ // multipartOptions: {
+ // limits: {
+ // fileSize: 1_000_000
+ // }
+ // }
+ // }
}, async function (req, reply) {
t.assert.fail('it should throw')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@is2ei
Got it. I tested it by changing the fileSize value to 2_500_000. Commenting out the configuration results in an error due to the default fileSize value, which is fastify.initialConfig.bodyLimit = 1_000_000.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If necessary, I can change the values to make it clearer what is causing the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test should be updated so that the error is caused by the config, as the message states:
should throw fileSize limitation error when used alongside attachFieldsToBody and set request config
There was a need to restrict the file size in each route when used alongside attachFieldsToBody. These changes will enable route-specific busboy configuration.
Example
Checklist
npm run test
andnpm run benchmark
and the Code of conduct