test(api): cover Kestrel request-body ceiling + upload-limit relationship - #381
Conversation
…lationship (#243) Assert the app-registered Kestrel MaxRequestBodySize resolves to 10 MB by reading the built IOptions<KestrelServerOptions>, so a regression that drops or unbounds the ceiling fails. Add a consistency guard that the transport ceiling stays above the 8 MB upload content-type max, plus a validator test proving a 9 MB upload is rejected by the content-type rule before the request-body limit is reached. Widen AddCookieAndKestrelLimits to internal (test project already has InternalsVisibleTo) so the test exercises the exact production wiring. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Code Review: PR #381 — test(api): cover Kestrel request-body ceiling + upload-limit relationship
Scope: PR #381 in thomasluizon/orbit-api
Recommendation: APPROVE
Summary
This PR closes a test-coverage gap flagged by a prior prod-readiness audit: it adds unit tests asserting the Kestrel MaxRequestBodySize (10 MB) is correctly registered and sits above the 8 MB upload content-type max, plus a SignUploadValidator case proving a 9 MB upload is rejected before the transport limit would ever be hit. The only production change is widening AddCookieAndKestrelLimits from private to internal so the new test can exercise the real wiring via the pre-existing InternalsVisibleTo("Orbit.Infrastructure.Tests") grant — no behavior change. The diff is small, self-contained, and does exactly what the PR description claims.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
None.
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | PASS — access-modifier widening is a no-op for exposure (target is only reachable from production startup and the new test); no auth/CORS/JWT/logging/webhook surface touched. |
| contract-aligner | N/A — no DTO, Controller route, or packages/shared type changed. |
Validation
| Check | Result |
|---|---|
| Build (dotnet) | N/A in CI — covered by separate required Build check |
| Tests (dotnet) | N/A in CI — covered by separate required Unit Tests check |
Verified by inspection: AddCookieAndKestrelLimits (src/Orbit.Api/Extensions/ServiceCollectionExtensions.Infrastructure.cs:203) registers exactly 10 * 1024 * 1024 for MaxRequestBodySize, matching the test's ExpectedMaxRequestBodyBytes. UploadContentTypes.MaxSizeBytes (src/Orbit.Application/Uploads/Common/UploadContentTypes.cs:5) is 8 * 1024 * 1024, confirming the 10 MB > 8 MB headroom the consistency-guard test asserts. No pre-existing test covered this config (grepped tests/ for MaxRequestBodySize/AddCookieAndKestrelLimits — only the new file matches), so this is genuinely new coverage, not a duplicate.
Deferred — N/A dimensions & files not verdicted
- Dimension 8 (DESIGN.md/AI-slop), 9 (Parity), 10 (i18n): N/A — no
apps/*files in this diff. - Dimension 11 (Contract drift): N/A — no DTO/Zod schema touched.
- Dimension 14 (FEATURES.md parity): N/A — no user-facing feature added/changed/removed; this is test coverage plus an internal-visibility change only.
- Cross-model second opinion (Phase 6 step 2): UNAVAILABLE — no
opencoderuntime in CI; moot here since no Critical finding survived to need it. - All three changed files (the visibility change, the new
RequestSizeLimitTests.cs, and theSignUploadCommandHandlerTests.csaddition) received a verdict above — nothing deferred.
What's good
- Test asserts against the real production wiring (
AddCookieAndKestrelLimits) rather than re-declaring the limit as a magic number, so a future regression in the actual Kestrel config will be caught. - The consistency-guard test (
RegisteredRequestBodyCeiling_LeavesHeadroomAboveUploadContentTypeMax) encodes the relationship between the two limits, not just each value in isolation — this is exactly the kind of interaction bug a single-limit test would miss. - Minimal, justified visibility change (
private→internal) using the project's existingInternalsVisibleTomechanism rather than reflection or a new public surface. - Clear, WHY-bearing assertion messages on both new test cases.
Recommendation
Approve as-is — no changes required.



What
Closes a
/prod-readinesstests-audit gap:MaxRequestBodySize(10 MB Kestrel ceiling) and its relationship to the 8 MB upload content-type max were untested.Changes
RequestSizeLimitTests(Orbit.Infrastructure.Tests/Configuration/):IOptions<KestrelServerOptions>and asserts the registeredMaxRequestBodySizeresolves to 10 MB — a regression that drops or unbounds the ceiling fails.UploadContentTypes.MaxSizeBytes(8 MB), so an oversize upload is rejected by the validator, not truncated by the transport.SignUploadValidatorTests: adds a case proving a 9 MB upload (between the 8 MB content-type max and the 10 MB request-body limit) is rejected by the content-type rule before the request-body limit is reached.AddCookieAndKestrelLimitswidenedprivate→internalso the test exercises the exact production wiring (the test project already carriesInternalsVisibleTo). No behavior change.The limits are consistent — 10 MB > 8 MB leaves headroom — so no limit values were changed.
Verification
dotnet build Orbit.slnx— 0 errors.Orbit.Infrastructure.Tests— 1966 passed.Orbit.Application.Tests— 2773 passed.Refs thomasluizon/orbit-ui-mobile#243