Update body limit configuration to support size units#55
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request updates the request body size limit configuration from a numeric value in kilobytes to a more flexible string-based format that supports size units (K, M, G, etc.). Critically, this PR also fixes a bug where the previous implementation was incorrectly treating the numeric value as bytes rather than kilobytes, resulting in a much more restrictive limit than documented.
Changes:
- Changed the
BodyLimitfield type fromuint64tostringto support unit-based size specifications (e.g., "1M", "4K", "10M") - Updated the command-line flag from
flag.Uint64toflag.Stringwith a new default value of "1M" - Modified the conditional check from numeric comparison to empty string check
- Removed the
fmt.Sprintfformatting since the string value is now passed directly to Echo's middleware
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| main.go | Updated the body-limit flag from uint64 to string type, changed default from 1000 to "1M", and updated help text to reflect the new format with examples |
| internal/server/server.go | Changed BodyLimit field in Options struct from uint64 to string, updated documentation to describe supported formats and units, and modified the middleware initialization to use the string value directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if opts.BodyLimit != "" { | ||
| router.Use(middleware.BodyLimit(opts.BodyLimit)) |
There was a problem hiding this comment.
The BodyLimit middleware can panic if given an invalid format string. Consider adding validation for the BodyLimit string format before passing it to the middleware, or wrapping the middleware call in error handling. Valid formats should match the pattern: a number followed by an optional unit (K, KB, M, MB, G, GB, etc.). Invalid input like "invalid", "10X", or "-5M" could cause panics at startup.
This pull request updates the way the server handles request body size limits by switching from a numeric value in kilobytes to a more flexible string-based format that supports various units (e.g., K, M, G). This allows users to specify body size limits in a more intuitive way and aligns the configuration across the codebase.
Body size limit configuration improvements:
BodyLimitfield in theOptionsstruct from auint64(kilobytes) to astringthat accepts values like4K,10M, or1G, with an empty string disabling the limit.BodyLimitdirectly, removing the need for formatting or conversion.body-limitfrom auint64to astring, updating the default value and help text to reflect the new, more flexible format.