feat(proxy): make the inbound body admission limit configurable - #4138
Conversation
MAX_DECOMPRESSED_BODY_BYTES was hard-coded at 256 MiB, and on the 922k-token opt-in window a session's serialized history crosses it. The request that crosses it is Codex's own remote-compaction request, so the session 413s on the one operation that would have shrunk it and cannot recover. Adds the opt-in `maxInboundBodyBytes`, shaped like `maxUpstreamBodyBytes`: omitted or 0 keeps today's 256 MiB. Every reader resolves through resolveInboundBodyLimitBytes(), which clamps to [1 MiB, 512 MiB]. The ceiling is mandatory rather than advisory: readBoundedJsonRequestBody materializes the body several times over, so peak memory is a multiple of whatever is admitted and an unbounded inbound cap would be a memory exhaustion lever. The schema keeps the outbound guard's `.catch(undefined)` degradation, which is exactly why the bound cannot live there. Bun's listener rejects an oversized body before fetch() runs, so maxRequestBodySize is now resolved from config at bind time instead of being pinned to the default; an out-of-range value is clamped with one startup warning. Also separates the two 413s a client can now see on this surface. #4112 gives an upstream size refusal `context_length_exceeded`; a local admission refusal now answers `inbound_body_too_large` with a message that names OpenCodex as the refuser, the observed and configured sizes, and the config key that moves the limit. The diagnostic inherits the thrown error's rule about keeping non-finite and untyped values out of the text.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
…sal shape Bun answers 413 and stops reading while the client is still uploading, so the write side can surface the refusal as a transport error rather than a response. Accept either shape for the refusal, and pair it with the admitted case at the same body size so the assertion stays non-vacuous: the old listener, pinned to MAX_DECOMPRESSED_BODY_BYTES, admitted this body under every configuration.
리뷰 · 우선순위 68 / 80이 PR은 이슈 #3573을 닫으려는 변경이다. 현재 이 PR이 하는 일은 그 상한을 설정 가능하게 만드는 것이다. 가장 큰 블록커는 base다. 이 PR의 base는 경로/심볼 - base 경로/심볼 - 경로/심볼 - 라인 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 72 / 80이 PR은 이슈 #3573을 닫는다. 지금 이 PR이 하는 일은 그 상한을 설정 가능하게 만드는 것이다. 이전에 올렸던 리뷰의 블록커는 해제되었다. #4127은 경로/심볼 - 경로/심볼 - 라인 - 경로/심볼 - 경로/심볼 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Summary
Closes #3573.
MAX_DECOMPRESSED_BODY_BYTESinsrc/server/request-decompress.tswas hard-coded at 256 MiB with no config lever. On the 922k-token opt-in window a session's serialized history crosses that limit, and the request that crosses it is Codex's own remote-compaction request — so the session gets413 ... Decompressed request body exceeds 268435456 byteson the one operation that would have shrunk it, and cannot recover.maxInboundBodyBytes, shaped like the existingmaxUpstreamBodyBytes: omitted or0keeps today's 256 MiB, so an unconfigured proxy admits exactly what it admits now.resolveInboundBodyLimitBytes(), which clamps to[1 MiB, 512 MiB]. An unbounded inbound cap would be a memory-exhaustion lever:readBoundedJsonRequestBodymaterializes the body several times over (retained wire bytes, decoded bytes, the decoded string, the re-encoded measurement copies, and the parsed object graph), so peak RSS is a multiple of whatever is admitted. 512 MiB is the value the issue asked for and the largest that keeps that multiple survivable. The bound deliberately lives in code rather than in the schema, because the schema mirrors the outbound guard's.catch(undefined)degradation and a config object can also be built without the schema at all.fetchruns, so the listener'smaxRequestBodySizeis now resolved from config at bind time instead of being pinned to the default; otherwise the opt-in would silently do nothing. It is fixed at bind time, so a change takes effect on restart, which the config doc and the docs-site row both state. An out-of-range value is clamped with one startup warning naming the value actually used./v1/responses,/v1/responses/compact, policy fallback's body clone,/v1/chat/completions,/v1/messages, images, and search — so one deployment does not end up with two different inbound limits. Management routes keep their own separateMANAGEMENT_JSON_BODY_MAX_BYTES.Improved 413 diagnostic. The parent PR (#4127, issue #4112) gives an upstream size refusal on this same surface HTTP 413 with
code: context_length_exceeded. Without a distinct code the two are indistinguishable while having opposite remedies — one means the provider will not take the turn, the other means the proxy never read it and a config key would have let it through. A local admission refusal now answerscode: inbound_body_too_largewith a message that names OpenCodex as the refuser, the observed and configured sizes, and the key that moves the limit. The wording avoids "context window"/"context length" on purpose, sinceclassifyErrortreats those as evidence of an upstream context verdict. The diagnostic also inherits the thrown error's existing rule about keeping non-finite and untyped values out of the text.Originally opened on top of #4127 because both PRs change the meaning of a 413 on this surface and the new code only makes sense against the classified one. #4127 has since merged, so this now targets
devdirectly and its diff is only the change described above.Verification
bun installwere NOT RUN, per explicit maintainer instruction for this round. Exact-head remote CI on this PR is the gate.git diff --check: clean.tests/usage/request-decompress.test.ts— a newconfigurable inbound body limit (Issue #3573)block pins the default for unconfigured/zero/negative/non-finite input, the raise, the hard ceiling (includingMAX_SAFE_INTEGERand 4 GiB), the floor, thatreadJsonRequestBodyhonors the resolved limit rather than the default, that an inbound refusal isinbound_body_too_largeand notcontext_length_exceeded, that a lower-bound measurement is not reported as an exact size, and that non-finite/untyped inputs stay out of the client-facing message.expectBodyLimitResponsehelper was updated to assert the new code and the new client-facing message while still pinning the thrown message, which keeps its provenance for the log.tests/server/server-request-body-size.test.ts— a configured 2 MiB limit rejects a 3 MiB body at the listener, which the previously fixed listener admitted through to the handler. Proven downward on purpose so CI does not allocate 257 MiB.maxInboundBodyBytesrow to the configuration reference next tomaxUpstreamBodyBytes.Checklist