✨ feat: Add Max Func to Limiter Middleware #3070
Conversation
luk3skyw4lker
requested review from
gaby,
sixcolors,
ReneWerner87 and
efectn
and removed request for
a team
|
Warning Rate limit exceeded@luk3skyw4lker has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 59 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new feature, Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3070 +/- ##
==========================================
+ Coverage 83.62% 83.65% +0.02%
==========================================
Files 115 115
Lines 8332 8335 +3
==========================================
+ Hits 6968 6973 +5
+ Misses 1041 1040 -1
+ Partials 323 322 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- docs/middleware/limiter.md (3 hunks)
- middleware/limiter/config.go (2 hunks)
- middleware/limiter/limiter_fixed.go (4 hunks)
- middleware/limiter/limiter_test.go (2 hunks)
Additional context used
LanguageTool
docs/middleware/limiter.md
[grammar] ~100-~100: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | 5 ...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
[grammar] ~101-~101: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
golangci-lint
middleware/limiter/limiter_test.go
[high] 22-22: G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand)
(gosec)
GitHub Check: lint
middleware/limiter/limiter_test.go
[failure] 22-22:
G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand) (gosec)
Additional comments not posted (13)
middleware/limiter/limiter_fixed.go (4)
29-30: Use the MaxCalculator function to dynamically calculate the max value.The
maxvalue is now calculated using theMaxCalculatorfunction, providing flexibility for dynamic rate limits based on the request context.
65-65: Ensure remaining hits calculation is accurate.The
remaininghits calculation logic is correct and ensures the correct number of remaining hits is returned.
73-73: Check hits exceed max value condition.The logic correctly checks if the current hits exceed the
maxvalue, ensuring proper rate limiting behavior.
101-101: Set RateLimit headers dynamically.The
X-RateLimit-Limit,X-RateLimit-Remaining, andX-RateLimit-Resetheaders are set dynamically based on the calculatedmaxvalue and remaining hits, ensuring accurate rate limit information is provided to the client.middleware/limiter/config.go (2)
21-26: Add MaxCalculator function to Config struct.The
MaxCalculatorfunction is added to theConfigstruct to allow dynamic calculation of the max requests supported by the rate limiter middleware.
112-116: Set default MaxCalculator function.The default
MaxCalculatorfunction is set to return theMaxvalue from the config if no function is provided, ensuring backward compatibility and default behavior.docs/middleware/limiter.md (5)
46-48: Add MaxCalculator example to documentation.The example demonstrates how to configure the
MaxCalculatorfunction to dynamically determine the max requests for the rate limiter middleware.
80-93: Document the dynamic limit feature with MaxCalculator.The new section accurately describes the dynamic limit feature using the
MaxCalculatorfunction and provides a clear example for users.
122-124: Add default MaxCalculator function to documentation.The default
MaxCalculatorfunction is correctly documented, ensuring users understand the default behavior.
101-101: Correct grammatical issue with 429 response.The word "429" should include an ordinal indicator.
- A function to calculate the max number of recent connections during `Expiration` seconds before sending a 429 response. + A function to calculate the max number of recent connections during `Expiration` seconds before sending a 429th response.Likely invalid or redundant comment.
Tools
LanguageTool
[grammar] ~101-~101: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
100-101: Correct grammatical issue with 429 response.The word "429" should include an ordinal indicator.
- Max number of recent connections during `Expiration` seconds before sending a 429 response. + Max number of recent connections during `Expiration` seconds before sending a 429th response.Likely invalid or redundant comment.
Tools
LanguageTool
[grammar] ~100-~100: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | 5 ...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
[grammar] ~101-~101: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
middleware/limiter/limiter_test.go (2)
5-5: Import necessary packages for testing.The necessary packages for testing are correctly imported.
18-63: Add test for MaxCalculator feature.The new test function
Test_Limiter_With_Max_Calculatorcorrectly tests the functionality of theMaxCalculatorfeature, ensuring it works as expected with concurrency checks.Tools
golangci-lint
[high] 22-22: G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand)
(gosec)
GitHub Check: lint
[failure] 22-22:
G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand) (gosec)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- middleware/limiter/limiter_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- middleware/limiter/limiter_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- docs/middleware/limiter.md (3 hunks)
- middleware/limiter/limiter_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- middleware/limiter/limiter_test.go
Additional context used
LanguageTool
docs/middleware/limiter.md
[grammar] ~101-~101: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | 5 ...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
[grammar] ~102-~102: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
Additional comments not posted (5)
docs/middleware/limiter.md (5)
46-48: LGTM!The addition of the
MaxCalculatorparameter in the example configuration is clear and aligns with the PR objectives.
81-94: LGTM!The documentation for dynamic limit calculation using the
MaxCalculatorparameter is clear and provides a useful example.
123-125: LGTM!The default implementation of the
MaxCalculatorparameter in theConfigDefaultsection is clear and consistent with the PR objectives.
101-101: Correct the grammatical issue.The word ‘429’ should have an ordinal indicator (‘th’).
- Max number of recent connections during `Expiration` seconds before sending a 429 response. + Max number of recent connections during `Expiration` seconds before sending a 429th response.Likely invalid or redundant comment.
Tools
LanguageTool
[grammar] ~101-~101: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | 5 ...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
102-102: Correct the grammatical issue.The word ‘429’ should have an ordinal indicator (‘th’).
- A function to calculate the max number of recent connections during `Expiration` seconds before sending a 429 response. + A function to calculate the max number of recent connections during `Expiration` seconds before sending a 429th response.Likely invalid or redundant comment.
Tools
LanguageTool
[grammar] ~102-~102: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
gaby
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- docs/middleware/limiter.md (3 hunks)
- middleware/limiter/limiter_sliding.go (3 hunks)
Additional context used
LanguageTool
docs/middleware/limiter.md
[grammar] ~101-~101: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | 5 ...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
[grammar] ~102-~102: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
Additional comments not posted (7)
middleware/limiter/limiter_sliding.go (3)
31-31: Dynamic calculation of max value using MaxCalculator.The introduction of
MaxCalculatorallows for dynamic calculation of the max value based on the request context, which enhances flexibility. This change aligns with the PR objectives and the linked issue's requirements.
34-34: Condition for middleware execution updated.The condition to skip middleware execution is updated to consider the dynamically calculated
maxvalue. This ensures that the middleware behaves correctly under the new configuration wheremaxcan be 0, effectively disabling the rate limiting.
132-132: Conversion of max value to string before setting the header.The
maxvalue is now correctly converted to a string usingstrconv.Itoa(max)before setting thexRateLimitLimitheader. This is necessary because HTTP headers must be strings. The change is well-implemented and ensures the correct operation of the middleware.docs/middleware/limiter.md (4)
46-48: Example usage of MaxCalculator in documentation.The documentation provides a clear example of how to use the
MaxCalculatorfeature. This example aligns with the new functionality introduced in the PR and helps users understand how to implement dynamic rate limiting.
89-93: Detailed example of dynamic limit calculation.This section provides a detailed example of using
MaxCalculatorto dynamically calculate limits based on the request's context. It's a valuable addition to the documentation as it guides users on implementing custom logic for rate limiting.
102-102: Update of MaxCalculator type in the Config table.The type of
MaxCalculatorhas been correctly updated in the documentation to reflect its function signature. This change ensures that the documentation is consistent with the code changes and provides accurate information to the users.Tools
LanguageTool
[grammar] ~102-~102: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
123-125: Default value for MaxCalculator in ConfigDefault.The documentation correctly shows the default implementation of
MaxCalculator, which returns a fixed value. This is important for users to understand the default behavior if they do not provide a custom function.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- middleware/limiter/config.go (2 hunks)
- middleware/limiter/limiter_fixed.go (4 hunks)
- middleware/limiter/limiter_sliding.go (3 hunks)
- middleware/limiter/limiter_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- middleware/limiter/limiter_fixed.go
- middleware/limiter/limiter_sliding.go
- middleware/limiter/limiter_test.go
Additional comments not posted (2)
middleware/limiter/config.go (2)
21-26: Addition ofMaxFuncfield is well-implemented.The new
MaxFuncfield allows for dynamic calculation of the maximum requests supported by the rate limiter middleware. This addition enhances flexibility.
112-116: Default implementation forMaxFuncensures backward compatibility.The
configDefaultfunction now includes a default implementation forMaxFuncthat returns the value ofMax. This approach maintains backward compatibility and avoids additional checks within the middleware.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- docs/middleware/limiter.md (3 hunks)
Additional context used
LanguageTool
docs/middleware/limiter.md
[grammar] ~101-~101: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | 5 ...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
[grammar] ~102-~102: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
Additional comments not posted (4)
docs/middleware/limiter.md (4)
46-48: Example forMaxFunclooks good!The example for the
MaxFuncparameter is clear and demonstrates how to use the new feature effectively.
81-94: Dynamic limit section is well-explained!The new section explaining the
MaxFuncparameter is clear and provides a good example. It helps users understand how to use the new feature effectively.
123-125: Default implementation forMaxFunclooks good!The default implementation for the
MaxFuncparameter is correct and follows best practices.
102-102: Correct the type ofMaxFuncin the Config table.The type of
MaxFuncshould befunc(fiber.Ctx) intinstead ofint.- | MaxFunc | `int` | A function to calculate the max number of recent connections during `Expiration` seconds before sending a 429 response. | A function which returns the cfg.Max | + | MaxFunc | `func(fiber.Ctx) int` | A function to calculate the max number of recent connections during `Expiration` seconds before sending a 429 response. | A function which returns the cfg.Max |Likely invalid or redundant comment.
Tools
LanguageTool
[grammar] ~102-~102: The word ‘429’ seems to be an ordinal number with a missing ordinal indicator (‘st’, ‘nd’, ‘rd’ or ‘th’).
Context: ...gExpirationseconds before sending a 429 response. | A function which returns th...(ORDINAL_NUMBER_MISSING_ORDINAL_INDICATOR)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- middleware/limiter/config.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- middleware/limiter/config.go
luk3skyw4lker
changed the title
* feat: add max calculator to limiter middleware * docs: update docs including the new parameter * refactor: add new line before go code in docs * fix: use crypto/rand instead of math/rand on tests * test: add new test with zero set as limit * fix: repeated tests failing when generating random limits * fix: wrong type of MaxCalculator in docs * feat: include max calculator in limiter_sliding * refactor: rename MaxCalculator to MaxFunc * docs: update docs with MaxFunc parameter * tests: rename tests and add test for limiter sliding
Description
This PR adds a MaxCalcualtor parameter to the limiter middleware, allowing it to calculate different rate limits for different requests, if no function is provided, the cfg.Max parameter is used as a returning value.
Fixes #3065
Type of change
Checklist
Before you submit your pull request, please make sure you meet these requirements:
/docs/directory for Fiber's documentation.Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md