Skip to content

Fixed: Prevent integer overflow when accumulating webfetch response sizes#44

Merged
Sewer56 merged 1 commit intomainfrom
fix/webfetch-overflow-safe
Feb 28, 2026
Merged

Fixed: Prevent integer overflow when accumulating webfetch response sizes#44
Sewer56 merged 1 commit intomainfrom
fix/webfetch-overflow-safe

Conversation

@Sewer56
Copy link
Copy Markdown
Member

@Sewer56 Sewer56 commented Feb 28, 2026

Summary

Fixed: Prevent integer overflow when accumulating webfetch response sizes

Security

  • Use checked_add in both tokio and blocking implementations to safely accumulate
    response sizes. Returns HTTP error instead of panicking or wrapping on overflow.

Performance

  • Switch blocking implementation to use 64 KiB BufReader with fill_buf/consume
    pattern instead of 8 KiB stack buffer for better I/O efficiency.

Files Changed

  • llm-coding-tools-core/src/tools/webfetch/tokio_impl.rs
  • llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs

…izes

Use checked_add in both tokio and blocking implementations to safely
accumulate response sizes. Returns HTTP error instead of panicking or
wrapping on overflow.

Also optimized blocking implementation with BufReader (64 KiB) using
fill_buf/consume pattern for better I/O performance.
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.22%. Comparing base (100ef7e) to head (2c7ecf8).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   75.16%   75.22%   +0.06%     
==========================================
  Files          67       67              
  Lines        2021     2026       +5     
==========================================
+ Hits         1519     1524       +5     
  Misses        502      502              
Flag Coverage Δ
unittests 75.22% <100.00%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ing-tools-core/src/tools/webfetch/blocking_impl.rs 92.50% <100.00%> (+0.60%) ⬆️
...coding-tools-core/src/tools/webfetch/tokio_impl.rs 92.10% <100.00%> (+0.43%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 100ef7e and 2c7ecf8.

📒 Files selected for processing (2)
  • src/llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs
  • src/llm-coding-tools-core/src/tools/webfetch/tokio_impl.rs
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Build and Test (Async/Tokio) (macos-latest, aarch64-apple-darwin, false)
  • GitHub Check: Build and Test (Async/Tokio) (windows-latest, x86_64-pc-windows-msvc, false)
  • GitHub Check: Build and Test (Async/Tokio) (ubuntu-latest, x86_64-unknown-linux-gnu, false)
  • GitHub Check: Build and Test (Blocking) (windows-latest, x86_64-pc-windows-msvc, false)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs

📄 CodeRabbit inference engine (src/AGENTS.md)

src/**/*.rs: Preallocate collections when size is known or estimable using String::with_capacity(estimated_len), Vec::with_capacity(count), or BufReader::with_capacity(size, reader)
Prefer &str / &[T] returns over owned types when lifetime allows
Use Cow<'_, str> for conditional ownership (e.g., String::from_utf8_lossy)
Use &'static str for compile-time constant strings
Reuse buffers by calling .clear() and reusing Vec/String instead of reallocating
Use const generics for compile-time branching (e.g., <const LINE_NUMBERS: bool>)
Use #[inline] on small, hot-path functions
Prefer core over std where possible (e.g., core::mem over std::mem)
Use memchr crate for fast byte searching over manual iteration
Keep modules under 500 lines (excluding tests); split if larger
Place use statements inside functions only for #[cfg] conditional compilation
Document public items with /// and add examples in docs where helpful
Use //! for module-level documentation
Use [TypeName] rustdoc links instead of backticks in documentation

Files:

  • src/llm-coding-tools-core/src/tools/webfetch/tokio_impl.rs
  • src/llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs
🧠 Learnings (2)
📚 Learning: 2026-02-07T22:53:26.067Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.067Z
Learning: Applies to src/**/*.rs : Reuse buffers by calling `.clear()` and reusing `Vec`/`String` instead of reallocating

Applied to files:

  • src/llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs
📚 Learning: 2026-02-07T22:53:26.067Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-02-07T22:53:26.067Z
Learning: Applies to src/**/*.rs : Preallocate collections when size is known or estimable using `String::with_capacity(estimated_len)`, `Vec::with_capacity(count)`, or `BufReader::with_capacity(size, reader)`

Applied to files:

  • src/llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs
🧬 Code graph analysis (1)
src/llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs (1)
src/llm-coding-tools-core/src/tools/webfetch/mod.rs (1)
  • check_size (46-54)
🔇 Additional comments (4)
src/llm-coding-tools-core/src/tools/webfetch/tokio_impl.rs (1)

61-63: LGTM! Safe overflow handling.

The checked_add approach correctly prevents integer overflow and returns a descriptive error. The check happens before extending the buffer, maintaining the fail-fast pattern.

src/llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs (3)

5-5: LGTM!

Correct imports for the BufRead trait and BufReader type needed for the buffered reading pattern.


55-56: LGTM! Good buffer sizing choice.

The 64 KiB buffer is a reasonable choice for network I/O, balancing memory usage with read efficiency. Using BufReader::with_capacity aligns with the coding guidelines for preallocating buffers.


58-74: LGTM! Idiomatic and safe buffered reading.

The fill_buf/consume pattern is the correct approach for BufRead:

  • Avoids redundant copies by reading directly into the internal buffer
  • Empty chunk correctly signals EOF
  • Overflow check with checked_add is consistent with the tokio implementation
  • Validation order (overflow → size check → copy) is correct

Walkthrough

Changes to the webfetch module improve buffer handling and add overflow detection in HTTP response streaming. The blocking implementation replaces synchronous Read-based streaming with BufReader-based streaming using a fixed 65536-byte buffer, utilizing fill_buf and consume APIs instead of direct chunk reads. The tokio implementation adds overflow checking using checked_add when accumulating response chunk sizes. Both implementations now return a ToolError on size overflow rather than proceeding with potentially invalid totals. Existing per-chunk size validation and Content-Length pre-checks are preserved.

Possibly related PRs

  • llm-coding-tools#41: Both PRs modify the webfetch blocking implementation's read loop to replace unsafe/uninitialized-buffer reads with safe buffered-read approaches using different buffering strategies.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and clearly describes the main security objective of the PR: preventing integer overflow when accumulating webfetch response sizes.
Description check ✅ Passed The description covers the key changes including security improvements (checked_add usage) and performance enhancements (BufReader buffer optimization), with affected files listed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/webfetch-overflow-safe

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.

@Sewer56 Sewer56 merged commit 373b4c9 into main Feb 28, 2026
9 checks passed
@Sewer56 Sewer56 deleted the fix/webfetch-overflow-safe branch February 28, 2026 12:34
Sewer56 added a commit that referenced this pull request Mar 30, 2026
Fixed: Prevent integer overflow when accumulating webfetch response sizes
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