fix(blaze): bound HTTP request bodies - #2293
Conversation
|
@codex review This PR bounds ordinary Blaze HTTP request bodies with a configurable default while preserving the existing guest-operation envelope. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a59f4fbd83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[serde(default)] | ||
| pub listen: ListenSection, | ||
| #[serde(default)] | ||
| pub api: ApiSection, |
There was a problem hiding this comment.
Document the public API limit field
DaemonConfig::api is a newly introduced public configuration field but has no rustdoc, leaving generated blaze-core API documentation without the field's ordinary-route limit and guest-envelope contract. Add a /// comment to this significant public field as required by the repository's Rust documentation rules.
AGENTS.md reference: AGENTS.md:L111-L115
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9c43a28d83b9. DaemonConfig::api now documents that this section applies to routes without a dedicated protocol envelope and that guest-operation routes retain their protocol-specific limit. Strict rustdoc passed for both default and all-feature builds on this exact commit.
Guest operation routes already used a bounded request envelope, but every other daemon route could still buffer an arbitrary body before dispatch. Add a validated api.max_body_bytes default for ordinary routes. Collect every request through a shared limit-aware reader. Reject malformed or conflicting Content-Length values before polling, and stop when streamed bytes first cross the selected route boundary. This gives ordinary requests a configurable 1 MiB default while preserving the existing 22 MiB guest envelope and its stable error code. Body format failures return 400, and size violations return 413. Document the route-specific behavior in the example and bilingual guides. Fixes: 4ec67cf ("feat(blaze): implement sandbox guest operations (alibaba#2217)") Signed-off-by: Weisson Han <wenshu.hx@linux.alibaba.com>
a59f4fb to
9c43a28
Compare
|
@codex review Please re-review the current head |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Why
Blaze already limits the HTTP envelope used by guest exec, read, and write operations, but every other daemon route could still buffer an arbitrary request body before dispatch. Ordinary API requests need a predictable per-request memory boundary without reducing the larger envelope required for guest file transfer.
What changed
Before:
After:
api.max_body_bytessetting, which defaults to 1 MiB.Content-Lengthbefore polling the body and stops when streamed bytes first cross the selected route limit.request_too_large.guest_request_too_largeresponse.Commit
9c43a28d83b9— bound daemon HTTP request bodies. Adds the configuration contract, shared collector, error mapping, focused tests, example, and bilingual documentation.These changes belong in one PR because they implement one request-collection boundary at the daemon entry point. The configuration selects that boundary, the collector enforces it, the API maps its result, and the tests and documentation define the same observable behavior. Removing this commit restores the previous unbounded ordinary-route behavior without changing sandbox lifecycle, storage, guest protocol, or backend contracts.
Related issue
fixes #2292
User / Agent impact
Existing configurations remain valid and receive the 1 MiB ordinary-route default. An ordinary request above that limit now receives HTTP 413 instead of being fully buffered; operators that intentionally send larger ordinary payloads can set a larger positive
api.max_body_bytesvalue.Guest command and file routes remain capped at 22 MiB independently of this setting, so their existing 16 MiB decoded file limit and error code do not change. The new boundary is per request; it does not cap aggregate memory across concurrent requests.
Risk and compatibility
The new configuration field is optional and defaults through serde, so existing files continue to parse. The observable compatibility change is that ordinary bodies above 1 MiB are rejected by default. Raise
api.max_body_bytesbefore deployment when an existing caller requires a larger ordinary payload. The value must be greater than zero.Validation
Exact commit:
9c43a28d83b98a3a2dee419dfdc6ad2e9d601d8aThe Git archive was copied to a fresh Linux x86_64 source directory. Its local and remote SHA-256 matched:
252cb5cf374634e4ca65b939108b9ae2336acc826f208688e7022067c3cb24f0The following checks passed with Rust/Cargo 1.93.1:
cargo fmt --all -- --checkcargo metadata --locked --format-version 1 --no-depscargo build --workspace --all-targets --lockedcargo build --workspace --all-targets --all-features --lockedcargo clippy --workspace --all-targets --locked -- -D warningscargo clippy --workspace --all-targets --all-features --locked -- -D warningscargo test --workspace --locked— 54 library + 119 daemon testscargo test --workspace --all-features --locked— 54 library + 129 daemon testsRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --lockedRUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features --lockedbash scripts/docs-lint.shpython3 scripts/docs-link-check.pygit diff --check, and public-boundary scanAll listed tests completed with zero failures. Hosted checks are reported by GitHub separately.
Documentation and rollback
The example configuration, component README in English and Chinese, and Blaze user guide in English and Chinese now document the default, override, response status, and guest-route exception. CHANGELOG aggregation remains reserved for a release version-bump PR.
To retain larger ordinary requests, set
api.max_body_bytesto a suitable positive byte count. Revert this commit to restore the previous ordinary-route collection behavior.