Skip to content

Fixed: Remove UB-prone uninit slice cast in blocking webfetch#41

Merged
Sewer56 merged 1 commit into
mainfrom
fix/blocking-webfetch-init-buffer
Feb 27, 2026
Merged

Fixed: Remove UB-prone uninit slice cast in blocking webfetch#41
Sewer56 merged 1 commit into
mainfrom
fix/blocking-webfetch-init-buffer

Conversation

@Sewer56
Copy link
Copy Markdown
Member

@Sewer56 Sewer56 commented Feb 27, 2026

Summary

  • Replace unsafe MaybeUninit slice casting in the blocking webfetch read loop with a safe initialized [0u8; 8192] buffer.
  • Keep chunked read/append behavior and response size checks unchanged.
  • Remove undefined behavior risk from creating &mut [u8] over uninitialized memory.

Basically, the old behaviour was technically correct.
But the slice constructor assumes that the data is 'initialized'.
I don't want to risk it with future compiler changes.

Use an initialized [0u8; 8192] stack buffer in the blocking read loop instead of casting MaybeUninit memory to [u8]. This removes UB risk while preserving chunked reads and size checks.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 27, 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 348806a and 272e0db.

📒 Files selected for processing (1)
  • src/llm-coding-tools-core/src/tools/webfetch/blocking_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). (5)
  • GitHub Check: Build and Test (Blocking) (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 (Async/Tokio) (windows-latest, x86_64-pc-windows-msvc, false)
  • GitHub Check: Build and Test (Blocking) (macos-latest, aarch64-apple-darwin, false)
  • GitHub Check: Build and Test (Async/Tokio) (macos-latest, aarch64-apple-darwin, 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/blocking_impl.rs
🧠 Learnings (3)
📓 Common learnings
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
📚 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 (2)
src/llm-coding-tools-core/src/tools/webfetch/blocking_impl.rs (2)

55-60: Safe read buffer change is correct and removes UB risk.

This replacement with a fully initialized [0u8; 8192] buffer and direct read(&mut buffer) is a solid safety improvement with equivalent behavior.


66-66: Chunk append remains correct and bounds-safe.

Using &buffer[..n] correctly appends only the bytes actually read.


Walkthrough

This change modifies the web fetch blocking implementation by replacing an unsafe MaybeUninit-based buffer with a direct [u8; 8192] stack-allocated byte array. The unsafe pointer arithmetic and raw pointer slice construction are removed in favor of directly calling read(&mut buffer). Data extraction adjusts to use &buffer[..n] instead. The refactoring preserves byte accumulation logic, content length validation, streaming until EOF, and error handling via ToolError::Http. No public API signatures are affected. The change results in a net reduction of 7 lines of code.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main change: removing undefined behavior from unsafe MaybeUninit casting in the blocking webfetch implementation.
Description check ✅ Passed The PR description covers the key changes and safety improvements, though it lacks detailed context about why this fix was necessary beyond UB elimination.
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/blocking-webfetch-init-buffer

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 4cac3b1 into main Feb 27, 2026
7 checks passed
@Sewer56 Sewer56 deleted the fix/blocking-webfetch-init-buffer branch February 27, 2026 22:08
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.84%. Comparing base (348806a) to head (272e0db).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
- Coverage   74.91%   74.84%   -0.07%     
==========================================
  Files          67       67              
  Lines        1985     1980       -5     
==========================================
- Hits         1487     1482       -5     
  Misses        498      498              
Flag Coverage Δ
unittests 74.84% <100.00%> (-0.07%) ⬇️

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 91.89% <100.00%> (-0.97%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sewer56 added a commit that referenced this pull request Mar 30, 2026
Fixed: Remove UB-prone uninit slice cast in blocking webfetch
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