-
Notifications
You must be signed in to change notification settings - Fork 293
feat: request body limit #1150
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?
feat: request body limit #1150
Conversation
- Create a plugin-based approach for body size limiting - Support route-specific limits with include/exclude patterns - Add comprehensive tests for the plugin - Addresses #859
Deploying h3dev with
|
| Latest commit: |
30c23e9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://76ed8f69.h3dev.pages.dev |
| Branch Preview URL: | https://feat-body-size-limit-plugin.h3dev.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
| * app.register(bodySizeLimit); | ||
| * ``` | ||
| */ | ||
| export function defineBodySizeLimitPlugin( |
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.
It should be a middleware not a plugin to apply in places needed.
With middleware, pattern and route matching can be defined also on definition therefore we do not need to (re)implement logic inside this utility.
| } | ||
|
|
||
| // Check Content-Length header first | ||
| const contentLength = event.req.headers.get("content-length"); |
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.
This is not a safe implementation as @OskarLebuda pointed out. content-length can be faked.
We need to both check header and transform body stream into another controller that force-stops reading body as soon as max length is reached.
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.
Thanks for PR ❤️ We need some changes:
- Implementation should return a middleware
- Implementation should not do pattern matching (middleware already do it)
- Implementation should also validate stream length other than simple header check
Summary
This PR introduces a new plugin system for limiting request body sizes in H3 applications, addressing the issue raised in #859.
Features
Usage
Why Plugin Approach?
Instead of adding
maxBodySizedirectly to route definitions, this plugin approach provides:Test Coverage
defineRouteCloses #859