fix: route body limit #36
Conversation
There was a problem hiding this comment.
Hi! Thank you for the quick PR.
I think this will fix my use case as reported in #35.
I hope you don't mine me adding a couple of comments.
test/route-limit.test.js
Outdated
| t.fail('body is not ok') | ||
| }) | ||
|
|
||
| await t.test('must not throw if body is smaller than limit', async t => { |
There was a problem hiding this comment.
I'd specify:
must not throw if body is smaller than route limit
test/route-limit.test.js
Outdated
| await t.test('must reject if body is bigger than limit', async t => { | ||
| const res = await app.inject({ | ||
| method: 'POST', | ||
| url: '/50', | ||
| payload | ||
| }) | ||
|
|
||
| t.equal(res.statusCode, 413) | ||
| t.same(res.json(), { | ||
| statusCode: 413, | ||
| code: 'FST_ERR_CTP_BODY_TOO_LARGE', | ||
| error: 'Payload Too Large', | ||
| message: 'Request body is too large' | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Why using different routes instead of different payloads?
I'd rather have just two routes:
route-limitserver-limit
And you test both with a bigger and a smaller payload
There was a problem hiding this comment.
I'd actually split route-limit into two tests: one were route-limit is bigger than server-limit and one where route-limit is smaller then server-limit.
This way you can test that the route level limit is always respected
plugin.js
Outdated
| @@ -56,16 +56,22 @@ function rawBody (fastify, opts, next) { | |||
| next() | |||
|
|
|||
| function preparsingRawBody (request, reply, payload, done) { | |||
| const applyLimit = request.context._parserOptions.limit || fastify.initialConfig.bodyLimit | |||
There was a problem hiding this comment.
should you check for null or undefined instead? is 0 technically a valid limit?
There was a problem hiding this comment.
Fastify rejects it
'bodyLimit' option must be an integer > 0. Got '0'
I fixed the check anyway by using the ?? operator instead
fix #35