Skip to content

fix: unify MAX_REQUEST_BODY_MB defaults and fallback handling - #3341

Open
Honghurumeng wants to merge 1 commit into
QuantumNous:mainfrom
Honghurumeng:main
Open

fix: unify MAX_REQUEST_BODY_MB defaults and fallback handling#3341
Honghurumeng wants to merge 1 commit into
QuantumNous:mainfrom
Honghurumeng:main

Conversation

@Honghurumeng

@Honghurumeng Honghurumeng commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Summary

统一 MAX_REQUEST_BODY_MB 的默认值与兜底逻辑,修正文档和运行时行为不一致的问题

当前项目中,MAX_REQUEST_BODY_MB 在文档、初始化代码以及请求体处理链路中的默认值和 <= 0 兜底行为并不一致。
本 PR 将这些逻辑统一为 128MB,避免出现配置无效时不同链路执行结果不一致的问题。

我在实际使用中128MB不够用,一直改到512MB才实现codex auto compact,但是考虑这个值能作为环境变量修改,就保持了128MB。

Background

项目中的请求体大小限制由环境变量 MAX_REQUEST_BODY_MB 控制,但此前存在两类不一致:

  1. 文档默认值与代码默认值不一致

    • README 中将 MAX_REQUEST_BODY_MB 默认值写为 32
    • 实际初始化代码中默认值为 128
  2. MAX_REQUEST_BODY_MB <= 0 时兜底行为不一致

    • 请求解压/入口限制中间件回退到 32MB
    • 请求体读取与缓存逻辑回退到 128MB

由于解压中间件和请求体读取逻辑都在实际请求链路中生效,这种不一致会导致配置为 0、负数或依赖默认值时,系统表现不稳定,也容易让文档使用者产生误解。

本 PR 统一了默认值来源和兜底逻辑,使其在所有相关代码路径中保持一致。

Changes

constant/env.go

新增共享默认值常量与统一兜底函数:

  • 增加 DefaultMaxRequestBodyMB = 128
  • 增加 EffectiveMaxRequestBodyMB(),用于统一处理 <= 0 时回退到默认值的逻辑

common/init.go

统一初始化默认值来源:

  • MAX_REQUEST_BODY_MB 的初始化默认值改为引用 constant.DefaultMaxRequestBodyMB
  • 避免默认值在不同文件中重复写死

middleware/gzip.go

统一请求解压/入口限制逻辑:

  • 改为通过 constant.EffectiveMaxRequestBodyMB() 获取有效限制值
  • 不再单独使用 32MB 作为 <= 0 的兜底值

common/gin.go

统一请求体读取与缓存限制逻辑:

  • 改为通过 constant.EffectiveMaxRequestBodyMB() 获取有效限制值
  • 与解压中间件保持一致,不再单独写死 128MB 回退逻辑

README.md

修正文档默认值:

  • MAX_REQUEST_BODY_MB 的默认值从 32 更新为 128
  • 使 README 与实际代码行为保持一致

constant/env_test.go

新增测试覆盖:

  • 验证正整数配置时按实际值生效
  • 验证 0 时回退到默认值 128
  • 验证负数时回退到默认值 128

Testing

  • 执行 gofmt 格式化相关 Go 文件
  • 本地执行:
    • go test ./constant ./common ./middleware
  • 验证结果:
    • constant 包测试通过
    • common 包测试通过
    • middleware 包当前无测试文件

Summary by CodeRabbit

  • Documentation

    • Updated the default maximum request body size limit from 32 MB to 128 MB, which is enforced after decompression and triggers HTTP 413 errors when exceeded.
  • Refactor

    • Centralized request body size limit handling.

@coderabbitai

coderabbitai Bot commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7e2dbcc4-0fa7-4fa4-b82b-d970b7181de2

📥 Commits

Reviewing files that changed from the base of the PR and between ed6ff0f and 6922e0f.

📒 Files selected for processing (6)
  • README.md
  • common/gin.go
  • common/init.go
  • constant/env.go
  • constant/env_test.go
  • middleware/gzip.go

Walkthrough

This PR refactors the request body size limit logic by centralizing the default value handling. A new constant DefaultMaxRequestBodyMB and function EffectiveMaxRequestBodyMB() encapsulate the defaulting behavior, replacing inline fallback logic previously scattered across initialization and request handling code. The documented default remains 128 MB.

Changes

Cohort / File(s) Summary
Documentation
README.md
Updated MAX_REQUEST_BODY_MB default value documentation from 32 MB to 128 MB.
Constants & Tests
constant/env.go, constant/env_test.go
Added DefaultMaxRequestBodyMB constant (128) and EffectiveMaxRequestBodyMB() function to centralize default-value logic; added comprehensive test coverage for the new function across positive, zero, and negative values.
Request Handling
common/gin.go, common/init.go, middleware/gzip.go
Refactored to delegate request-size limit defaulting to EffectiveMaxRequestBodyMB() instead of inline fallback logic; updated initialization to use new DefaultMaxRequestBodyMB constant.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • seefs001

Poem

🐰 A constant tucked in env.go's home,
No more scattered defaults to roam!
EffectiveMaxRequestBodyMB hops along,
One function to rule them all—clean and strong!
128 MB now truly known,
In one place only—neatly shown!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: unify MAX_REQUEST_BODY_MB defaults and fallback handling' accurately describes the main purpose of the PR: consolidating inconsistent defaults and fallback logic for the MAX_REQUEST_BODY_MB environment variable across multiple files into a single, unified approach.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can approve the review once all CodeRabbit's comments are resolved.

Enable the reviews.request_changes_workflow setting to automatically approve the review once all CodeRabbit's comments are resolved.

@ghost

This comment was marked as spam.

@Calcium-Ion
Calcium-Ion force-pushed the main branch 2 times, most recently from 51fdfc5 to 2b6f1df Compare August 30, 2026 15:03
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.

1 participant