http_server: enforce buffer_max_size for HTTP/2 request bodies#11521
http_server: enforce buffer_max_size for HTTP/2 request bodies#11521
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
📝 WalkthroughWalkthroughIntroduces a new internal helper function to validate HTTP/2 request body size limits. The function checks if appending data would exceed the configured maximum body size and is integrated at header parsing and data chunk reception points to abort processing when limits are exceeded. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/http_server/flb_http_server_http2.c`:
- Around line 93-95: The branch that checks for a NULL server currently returns
FLB_FALSE (disabling body-size enforcement) which is inconsistent with the
earlier fail-closed behavior for a missing parent_session; update the server
NULL check in flb_http_server_http2.c so that when server == NULL the function
returns FLB_TRUE to enforce body-size limits (mirroring the parent_session
handling) — locate the NULL check for "server" and change the return value from
FLB_FALSE to FLB_TRUE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1593641f-344e-4112-9e27-b9fbe4a41916
📒 Files selected for processing (1)
src/http_server/flb_http_server_http2.c
| if (server == NULL) { | ||
| return FLB_FALSE; | ||
| } |
There was a problem hiding this comment.
Fail-closed check should also apply when server is missing.
At Line 93-Line 95, server == NULL returns FLB_FALSE, which disables body-size enforcement in this error path. Since Line 87-Line 89 already fails closed for missing parent_session, this branch should be consistent to avoid unbounded body growth if linkage is incomplete.
Proposed fix
- if (server == NULL) {
- return FLB_FALSE;
- }
+ if (server == NULL) {
+ return FLB_TRUE;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (server == NULL) { | |
| return FLB_FALSE; | |
| } | |
| if (server == NULL) { | |
| return FLB_TRUE; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/http_server/flb_http_server_http2.c` around lines 93 - 95, The branch
that checks for a NULL server currently returns FLB_FALSE (disabling body-size
enforcement) which is inconsistent with the earlier fail-closed behavior for a
missing parent_session; update the server NULL check in flb_http_server_http2.c
so that when server == NULL the function returns FLB_TRUE to enforce body-size
limits (mirroring the parent_session handling) — locate the NULL check for
"server" and change the return value from FLB_FALSE to FLB_TRUE.
There was a problem hiding this comment.
@edsiper Could you take a look on this review comment? It could be mistakenly specified the returned value. This could cause something like operation glitches for disabling for body-size enforcement in this failing path.
is this intentional?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
Thank you for your quick fix |
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit