feat: (PRO-412) Cap max body size per request to prevent potential ov…

[dev-jodee]

…erloading of the server

---

Important

Introduces a feature to cap request body size at 2 MB to prevent server overload, with configuration and tests to ensure compliance.

• Behavior:
  • Adds max_request_body_size to KoraConfig in config.rs with a default of 2 MB.
  • Configures server in server.rs to enforce max_request_body_size.
  • New test test_request_body_oversized_rejected in body_size_limit.rs to ensure oversized requests return 413.
• Constants:
  • Defines DEFAULT_MAX_REQUEST_BODY_SIZE in constant.rs as 2 MB.
• Tests:
  • Adds body_size_limit.rs to test oversized request handling.
  • Updates config_mock.rs and toml_mock.rs to support max_request_body_size configuration.
  • Adds tests in config.rs to verify default and custom max_request_body_size.

^{This description was created by for b3da35e. You can customize this summary. It will automatically update as commits are pushed.}

📊 Unit Test Coverage

Unit Test Coverage: 85.4%

View Detailed Coverage Report

[linear]

PRO-412 Cap the memory the "body" of request can have to prevent potential DDOS

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to b3da35e in 1 minute and 33 seconds. Click for details.

• Reviewed 234 lines of code in 8 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. tests/adversarial/body_size_limit.rs:24

• Draft comment:
  Consider replacing unwrap() with expect() to provide clearer error messages in the test, ensuring that test failures communicate the issue better.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While expect() generally provides better error messages than unwrap(), this is a test file where the assertion already has a clear error message. The unwrap() is only used to get the status code, and if it fails, the test would fail with a panic that clearly indicates the response couldn't be obtained. The assert_eq! already has a descriptive message. This seems like a minor stylistic preference rather than a significant improvement. The comment does have merit since expect() is generally considered better practice than unwrap(). The change would make errors slightly more descriptive if the request fails to complete. However, the existing error handling is already adequate for a test file, with a clear assertion message. The suggested change would be a minor improvement at best. Delete the comment as it suggests a minor stylistic change that doesn't meaningfully improve the test's clarity or functionality.

2. tests/adversarial/main.rs:8

• Draft comment:
  Ensure that the adversarial tests (including body size limits) are isolated and do not impact other integration tests, especially when using shared configuration.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

3. tests/adversarial/main.rs:8

• Draft comment:
  Typo: Consider using "DDoS" instead of "DDOS" in the comment for consistency and correctness.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While technically "DDoS" is the more common capitalization, this is an extremely minor stylistic issue in a comment. The meaning is completely clear either way. The rules explicitly state not to make purely informative comments that don't require code changes. This seems like unnecessary nitpicking. The correct capitalization could be considered part of maintaining professional documentation standards. Some teams might care about this level of consistency. The rules are very clear that we should not make purely informative comments. The meaning is clear regardless of capitalization, and this creates unnecessary noise in the review. Delete this comment as it's purely stylistic, doesn't affect functionality, and violates the rule about not making purely informative comments.

Workflow ID: wflow_Yn10b06uEbFwIS1U

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Casting max_request_body_size from usize to u32 assumes the value is within u32 bounds. Consider adding an explicit check or documenting the limitation.

[amilz]

agree the suggestion from ellips maekes sense but lgtm