Skip to content

add Request Body Size Limiter #854

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

Merged
merged 17 commits into from
Jun 23, 2025
Merged

Conversation

jranson
Copy link
Member

@jranson jranson commented Jun 21, 2025

This patch fixes #808 and #806 by centralizing reading of the request body to a middleware handler that also implements the size limiter using an io.LimitReader as part of the centralized read. The default max request body size is 10MB but can be udpated in the config yaml. This patch also addresses POST-related defects in the ALB due to incompatibilities between the various non-centralized, slightly different DRY body reading implementations that are now refactored.

jranson added 6 commits June 20, 2025 10:15
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
@jranson jranson requested a review from crandles June 21, 2025 05:13
Signed-off-by: James Ranson <[email protected]>
@coveralls
Copy link

coveralls commented Jun 21, 2025

Pull Request Test Coverage Report for Build 15811013793

Details

  • 86 of 175 (49.14%) changed or added relevant lines in 19 files are covered.
  • 5 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.2%) to 76.335%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/daemon/setup/setup.go 0 1 0.0%
pkg/proxy/engines/proxy_request.go 21 23 91.3%
pkg/proxy/handlers/influxdb/influxdb.go 0 2 0.0%
pkg/backends/alb/mech/tsm/time_series_merge.go 6 9 66.67%
pkg/frontend/options/options.go 14 17 82.35%
pkg/proxy/handlers/clickhouse/clickhouse.go 0 3 0.0%
pkg/proxy/handlers/prometheus/prometheus.go 0 3 0.0%
pkg/proxy/handlers/rpc/rpc.go 0 3 0.0%
pkg/proxy/handlers/trickster/failures/failures.go 0 7 0.0%
pkg/proxy/response/merge/merge.go 0 9 0.0%
Files with Coverage Reduction New Missed Lines %
pkg/frontend/options/options.go 1 67.57%
pkg/proxy/handlers/clickhouse/clickhouse.go 1 0.0%
pkg/proxy/handlers/influxdb/influxdb.go 1 0.0%
pkg/proxy/handlers/prometheus/prometheus.go 1 0.0%
pkg/proxy/handlers/rpc/rpc.go 1 0.0%
Totals Coverage Status
Change from base Build 15785210143: -0.2%
Covered Lines: 15315
Relevant Lines: 20063

💛 - Coveralls

jranson added 6 commits June 21, 2025 02:24
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
jranson added 4 commits June 22, 2025 15:27
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
Signed-off-by: James Ranson <[email protected]>
@jranson jranson requested a review from crandles June 22, 2025 21:34
@jranson jranson merged commit 753d85e into trickstercache:main Jun 23, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

net/http.Request.*Body: consider usage vs custom pkg/proxy/request.*Body
3 participants