Request size issue fix#303
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts HTTP client configuration for VNext API services to support larger request and response sizes by increasing buffering and header limits on both configured clients. Class diagram for updated RemoteServiceExtensions HTTP client setupclassDiagram
class RemoteServiceExtensions {
+AddVNextApiServices IServiceCollection AddVNextApiServices(IServiceCollection services, VNextOptions options)
}
class HttpClient {
+int MaxResponseContentBufferSize
+HttpRequestHeaders DefaultRequestHeaders
}
class HttpClientHandler {
+DecompressionMethods AutomaticDecompression
+int MaxResponseHeadersLength
+long MaxRequestContentBufferSize
}
class IServiceCollection
class VNextOptions
RemoteServiceExtensions ..> IServiceCollection : extends
RemoteServiceExtensions ..> VNextOptions : configures
RemoteServiceExtensions ..> HttpClient : configures
RemoteServiceExtensions ..> HttpClientHandler : configures
HttpClient o-- HttpClientHandler : uses as primary handler
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note
|
| Cohort / File(s) | Change Summary |
|---|---|
HTTP Client Buffer & Header Configurationsrc/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs |
Increases HTTP client buffering limits: MaxResponseContentBufferSize set to int.MaxValue in both AddVNextApiServices setups, MaxResponseHeadersLength enabled to 1 MB, and MaxRequestContentBufferSize set to int.MaxValue in HttpClientHandler. Minor formatting: trailing comma added after AutomaticDecompression assignment. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
🐰 With buffers expanded to limits so vast,
The HTTP pipes flow unsurpassed!
Headers and responses now soar without bounds,
This rabbit rejoices at these config sounds! 📡✨
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title 'Request size issue fix' directly relates to the main change: increasing HTTP buffer and header size limits to resolve request size constraints. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
302-request-size
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 @coderabbitai help to get the list of available commands and usage tips.
Summary of ChangesHello @tsimsekburgan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses potential issues with large data transfers by significantly increasing the buffer and header size limits for HTTP client configurations. This ensures that the application can handle requests and responses with substantial content and header sizes without encountering truncation or errors, thereby improving the robustness of remote service interactions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Here's the code health analysis summary for commits Analysis Summary
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- Using
int.MaxValueforMaxResponseContentBufferSizeandMaxRequestContentBufferSizecan create significant memory pressure or DOS risk; consider setting these to a justified upper bound or making them configurable instead of effectively unbounded. - The comment for
MaxRequestContentBufferSizesays the default is 2GB while the code sets it toint.MaxValue(~2GB as well); double-check whether this change actually addresses the issue or if the comment/value should be adjusted for clarity. - The new handler and client configuration is duplicated in both
AddVNextApiServicesregistrations; consider extracting the common configuration into a shared helper to avoid divergence between the two clients.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `int.MaxValue` for `MaxResponseContentBufferSize` and `MaxRequestContentBufferSize` can create significant memory pressure or DOS risk; consider setting these to a justified upper bound or making them configurable instead of effectively unbounded.
- The comment for `MaxRequestContentBufferSize` says the default is 2GB while the code sets it to `int.MaxValue` (~2GB as well); double-check whether this change actually addresses the issue or if the comment/value should be adjusted for clarity.
- The new handler and client configuration is duplicated in both `AddVNextApiServices` registrations; consider extracting the common configuration into a shared helper to avoid divergence between the two clients.
## Individual Comments
### Comment 1
<location> `src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs:61-62` </location>
<code_context>
client.DefaultRequestHeaders.Add(clientOptions.InternalOperationHeader, "true");
}
+
+ // Increase buffer size limits to handle large responses
+ client.MaxResponseContentBufferSize = int.MaxValue;
})
.ConfigurePrimaryHttpMessageHandler(() => new HttpClientHandler()
</code_context>
<issue_to_address>
**issue:** Setting MaxResponseContentBufferSize to int.MaxValue is likely a no-op and may be misleading.
On current .NET versions `HttpClient.MaxResponseContentBufferSize` already defaults to `int.MaxValue`, so this line likely has no effect while the comment implies a behavioral change. Consider removing the comment (and possibly the assignment), or binding this value to configuration so any change is intentional and maintainable.
</issue_to_address>
### Comment 2
<location> `src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs:69-70` </location>
<code_context>
+ AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate,
+ // Increase response headers length limit (default is 64KB)
+ MaxResponseHeadersLength = 1024 * 1024, // 1MB
+ // Increase request content buffer size limit (default is 2GB)
+ MaxRequestContentBufferSize = int.MaxValue
})
.AddPolicyHandler(GetTimeoutPolicy(options))
</code_context>
<issue_to_address>
**issue:** MaxRequestContentBufferSize assignment and comment seem inconsistent and potentially ineffective.
Because `int.MaxValue` is roughly 2GB, this likely doesn’t change the default behavior despite the comment suggesting an increase. On newer .NET versions this property may also be deprecated/ignored. Consider removing it if redundant, or set it to a clearly justified, configurable limit instead of `int.MaxValue`.
</issue_to_address>
### Comment 3
<location> `src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs:67-68` </location>
<code_context>
{
- AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate
+ AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate,
+ // Increase response headers length limit (default is 64KB)
+ MaxResponseHeadersLength = 1024 * 1024, // 1MB
+ // Increase request content buffer size limit (default is 2GB)
+ MaxRequestContentBufferSize = int.MaxValue
</code_context>
<issue_to_address>
**🚨 issue (security):** MaxResponseHeadersLength unit mismatch makes the effective limit far larger than intended.
`MaxResponseHeadersLength` is in kilobytes, so `1024 * 1024` sets the limit to ~1GB, not 1MB, which significantly increases memory exposure and DoS risk. If you intend a 1MB limit, use `MaxResponseHeadersLength = 1024`, and consider using the smallest value that still covers your known scenarios.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // Increase buffer size limits to handle large responses | ||
| client.MaxResponseContentBufferSize = int.MaxValue; |
There was a problem hiding this comment.
issue: Setting MaxResponseContentBufferSize to int.MaxValue is likely a no-op and may be misleading.
On current .NET versions HttpClient.MaxResponseContentBufferSize already defaults to int.MaxValue, so this line likely has no effect while the comment implies a behavioral change. Consider removing the comment (and possibly the assignment), or binding this value to configuration so any change is intentional and maintainable.
| // Increase request content buffer size limit (default is 2GB) | ||
| MaxRequestContentBufferSize = int.MaxValue |
There was a problem hiding this comment.
issue: MaxRequestContentBufferSize assignment and comment seem inconsistent and potentially ineffective.
Because int.MaxValue is roughly 2GB, this likely doesn’t change the default behavior despite the comment suggesting an increase. On newer .NET versions this property may also be deprecated/ignored. Consider removing it if redundant, or set it to a clearly justified, configurable limit instead of int.MaxValue.
| // Increase response headers length limit (default is 64KB) | ||
| MaxResponseHeadersLength = 1024 * 1024, // 1MB |
There was a problem hiding this comment.
🚨 issue (security): MaxResponseHeadersLength unit mismatch makes the effective limit far larger than intended.
MaxResponseHeadersLength is in kilobytes, so 1024 * 1024 sets the limit to ~1GB, not 1MB, which significantly increases memory exposure and DoS risk. If you intend a 1MB limit, use MaxResponseHeadersLength = 1024, and consider using the smallest value that still covers your known scenarios.
There was a problem hiding this comment.
Code Review
This pull request aims to resolve issues with large request sizes by adjusting HttpClient settings. While the increase of MaxResponseHeadersLength is a valid change, the modifications to MaxResponseContentBufferSize and MaxRequestContentBufferSize are problematic. Both are set to their default values, rendering the changes ineffective. Furthermore, MaxRequestContentBufferSize is an obsolete property. Relying on large in-memory buffers for requests and responses is a significant performance and stability risk, potentially leading to OutOfMemoryException. I have provided comments to remove the ineffective code. Additionally, there is significant code duplication in the HttpClient setup that should be refactored to improve maintainability.
| // Increase request content buffer size limit (default is 2GB) | ||
| MaxRequestContentBufferSize = int.MaxValue |
There was a problem hiding this comment.
| // Increase request content buffer size limit (default is 2GB) | ||
| MaxRequestContentBufferSize = int.MaxValue |
| // Increase buffer size limits to handle large responses | ||
| client.MaxResponseContentBufferSize = int.MaxValue; |
There was a problem hiding this comment.
Setting MaxResponseContentBufferSize to int.MaxValue is redundant as this is already the default value. While this change has no effect, it's important to note that relying on buffering for large responses can lead to high memory consumption and potential OutOfMemoryException. A more robust approach for large payloads is to stream the response content.
| // Increase buffer size limits to handle large responses | ||
| client.MaxResponseContentBufferSize = int.MaxValue; |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs:
- Around line 99-103: The MaxResponseHeadersLength value is using bytes (1024 *
1024) but the property expects kilobytes, so in the RemoteServiceExtensions
configuration replace the current expression (shown with
MaxResponseHeadersLength) with 1024 to represent 1MB and update the inline
comment accordingly; keep AutomaticDecompression and MaxRequestContentBufferSize
as-is and ensure the comment notes the unit is kilobytes.
- Around line 66-70: The MaxResponseHeadersLength is expressed in kilobytes, so
replace the incorrect value (currently 1024 * 1024) with 1024 to enforce a ~1MB
header limit by setting MaxResponseHeadersLength = 1024; also remove the
MaxRequestContentBufferSize setter (MaxRequestContentBufferSize) because it is
ignored on .NET Core/5+ (including net9.0) so delete that line rather than
relying on a no-op, and leave the AutomaticDecompression line unchanged.
🧹 Nitpick comments (2)
src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs (2)
60-62: Consider making buffer size limits configurable and document memory implications.Setting
MaxResponseContentBufferSizetoint.MaxValue(~2GB) could cause significant memory pressure if multiple concurrent requests receive large responses. Consider:
- Adding configurable buffer size options to
RemoteOptionsinstead of hardcodingint.MaxValue- Documenting the expected response sizes that necessitated this change
♻️ Suggested approach: Add configurable options
Add to
RemoteOptions:public long MaxResponseContentBufferSize { get; set; } = 100 * 1024 * 1024; // 100MB defaultThen use it in configuration:
- client.MaxResponseContentBufferSize = int.MaxValue; + client.MaxResponseContentBufferSize = clientOptions.MaxResponseContentBufferSize;
93-95: Duplicated configuration - consider extracting to a shared helper.This configuration block is identical to lines 60-62. Extracting HTTP client configuration to a helper method would reduce duplication and ensure both clients stay in sync.
♻️ Suggested extraction
private static void ConfigureHttpClient(HttpClient client, IRuntimeInfoProvider runtimeInfoProvider, RemoteOptions clientOptions) { client.Timeout = TimeSpan.FromSeconds(clientOptions.TimeoutSeconds); client.DefaultRequestHeaders.Add("Accept", "application/json"); client.DefaultRequestHeaders.Add("User-Agent", $"vnext-runtime/{runtimeInfoProvider.Version} ({runtimeInfoProvider.Domain})"); if (clientOptions.EnableCircuitBreakerBypass) { client.DefaultRequestHeaders.Add(clientOptions.InternalOperationHeader, "true"); } client.MaxResponseContentBufferSize = int.MaxValue; // or clientOptions.MaxResponseContentBufferSize }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.cursor/rules/vnext.mdc)
**/*.cs: Write concise, idiomatic C# code with accurate examples
Follow Aether Framework's recommended folder and module structure (*.Application, *.Domain, *.Infrastructure, *.HttpApi)
Use object-oriented and functional programming patterns as appropriate
Prefer LINQ and lambda expressions for collection operations
Use descriptive variable and method names (e.g., IsUserSignedIn, CalculateTotal)
Follow Microsoft's modular development approach with extension structure to separate concerns between layers (Application, Domain, Infrastructure, etc.)
Organize extensions following the namespace of the class or structure where the extension will be written, generally in BBT.Workflow.Domain project to minimize 'using' statements
Follow Clean Architecture and SOLID principles
Apply Domain-Driven Design patterns: Aggregates, Entities, ValueObjects, Repositories, Domain Events
All cross-cutting concerns such as Clock, GuidGenerator, Mapper, Tracing, Logging, Metrics MUST use Aether SDK
Workflow, orchestration, task handling and runtime logic MUST follow vNext architectural conventions
Always use async/await
Use Dependency Injection everywhere
Avoid leaking EF entities
Apply Result Pattern
Use PascalCase for class names, method names, and public members
Use camelCase for local variables and private fields
Use UPPERCASE for constants
Prefix interface names with 'I' (e.g., IUserService)
Use C# 10+ features when appropriate (e.g., record types, pattern matching, null-coalescing assignment)
Leverage built-in ASP.NET Core features and middleware, as well as Aether's modules and features
Use Entity Framework Core effectively for database operations, integrating with Aether's AetherDbContext and repository abstractions
Follow the C# Coding Conventions from Microsoft documentation
Use C#'s expressive syntax (e.g., null-conditional operators, string interpolation)
Use 'var' for implicit typing when the type is obvious
Use exceptions for exceptional cases, not for control flow
Implemen...
Files:
src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Sourcery review
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Build and analyze
| AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate, | ||
| // Increase response headers length limit (default is 64KB) | ||
| MaxResponseHeadersLength = 1024 * 1024, // 1MB | ||
| // Increase request content buffer size limit (default is 2GB) | ||
| MaxRequestContentBufferSize = int.MaxValue |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
HttpClientHandler.MaxResponseHeadersLength unit documentation .NET
💡 Result:
The HttpClientHandler.MaxResponseHeadersLength value is measured in kilobytes (KB). The .NET docs note the default maximum is 64 KB and warn that the property is in kilobytes (not bytes). [1][2]
Sources:
- Microsoft Learn: CA2262 (explains the property is measured in kilobytes; default 64 KB). [1]
- .NET HttpClient documentation (context for HttpClient/handlers). [2]
🌐 Web query:
HttpClientHandler.MaxRequestContentBufferSize default value .NET
💡 Result:
Short answer: On .NET Core (and UWP) HttpClientHandler.MaxRequestContentBufferSize is not supported — the getter returns 0 and the setter is ignored. On older .NET Framework the property existed for buffering behavior. (gist.github.com)
Citations:
🏁 Script executed:
find . -name "*.csproj" -o -name "*.sln" | head -20Repository: burgan-tech/vnext
Length of output: 1326
🏁 Script executed:
fd -e csproj | grep -i "Infrastructure\|Workflow" | head -10Repository: burgan-tech/vnext
Length of output: 784
🏁 Script executed:
cat src/BBT.Workflow.Infrastructure/BBT.Workflow.Infrastructure.csprojRepository: burgan-tech/vnext
Length of output: 1454
🏁 Script executed:
cat -n src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs | sed -n '50,80p'Repository: burgan-tech/vnext
Length of output: 2044
Critical: MaxResponseHeadersLength unit is in kilobytes, not bytes.
MaxResponseHeadersLength is measured in kilobytes, not bytes. Setting 1024 * 1024 actually configures ~1GB (1,048,576 KB), not 1MB as the comment claims.
For 1MB response headers limit, use 1024:
🐛 Proposed fix
AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate,
- // Increase response headers length limit (default is 64KB)
- MaxResponseHeadersLength = 1024 * 1024, // 1MB
+ // Increase response headers length limit (default is 64KB)
+ MaxResponseHeadersLength = 1024, // 1MB (unit is KB)
// Increase request content buffer size limit (default is 2GB)
MaxRequestContentBufferSize = int.MaxValueAdditionally, MaxRequestContentBufferSize is not supported on .NET Core/5+ (including net9.0 used here)—the setter is ignored at runtime. Remove this line unless documenting intent for a future migration.
📝 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.
| AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate, | |
| // Increase response headers length limit (default is 64KB) | |
| MaxResponseHeadersLength = 1024 * 1024, // 1MB | |
| // Increase request content buffer size limit (default is 2GB) | |
| MaxRequestContentBufferSize = int.MaxValue | |
| AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate, | |
| // Increase response headers length limit (default is 64KB) | |
| MaxResponseHeadersLength = 1024, // 1MB (unit is KB) | |
| // Increase request content buffer size limit (default is 2GB) | |
| MaxRequestContentBufferSize = int.MaxValue |
🤖 Prompt for AI Agents
In @src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs
around lines 66 - 70, The MaxResponseHeadersLength is expressed in kilobytes, so
replace the incorrect value (currently 1024 * 1024) with 1024 to enforce a ~1MB
header limit by setting MaxResponseHeadersLength = 1024; also remove the
MaxRequestContentBufferSize setter (MaxRequestContentBufferSize) because it is
ignored on .NET Core/5+ (including net9.0) so delete that line rather than
relying on a no-op, and leave the AutomaticDecompression line unchanged.
| AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate, | ||
| // Increase response headers length limit (default is 64KB) | ||
| MaxResponseHeadersLength = 1024 * 1024, // 1MB | ||
| // Increase request content buffer size limit (default is 2GB) | ||
| MaxRequestContentBufferSize = int.MaxValue |
There was a problem hiding this comment.
Same MaxResponseHeadersLength unit issue as above.
Apply the same fix here - the unit is kilobytes, so use 1024 for 1MB:
🐛 Proposed fix
AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate,
- // Increase response headers length limit (default is 64KB)
- MaxResponseHeadersLength = 1024 * 1024, // 1MB
+ // Increase response headers length limit (default is 64KB)
+ MaxResponseHeadersLength = 1024, // 1MB (unit is KB)
// Increase request content buffer size limit (default is 2GB)
MaxRequestContentBufferSize = int.MaxValue📝 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.
| AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate, | |
| // Increase response headers length limit (default is 64KB) | |
| MaxResponseHeadersLength = 1024 * 1024, // 1MB | |
| // Increase request content buffer size limit (default is 2GB) | |
| MaxRequestContentBufferSize = int.MaxValue | |
| AutomaticDecompression = System.Net.DecompressionMethods.GZip | System.Net.DecompressionMethods.Deflate, | |
| // Increase response headers length limit (default is 64KB) | |
| MaxResponseHeadersLength = 1024, // 1MB (unit is KB) | |
| // Increase request content buffer size limit (default is 2GB) | |
| MaxRequestContentBufferSize = int.MaxValue |
🤖 Prompt for AI Agents
In @src/BBT.Workflow.Infrastructure/Remote/Extensions/RemoteServiceExtensions.cs
around lines 99 - 103, The MaxResponseHeadersLength value is using bytes (1024 *
1024) but the property expects kilobytes, so in the RemoteServiceExtensions
configuration replace the current expression (shown with
MaxResponseHeadersLength) with 1024 to represent 1MB and update the inline
comment accordingly; keep AutomaticDecompression and MaxRequestContentBufferSize
as-is and ensure the comment notes the unit is kilobytes.
|


Summary by Sourcery
Bug Fixes:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.