Fix TeeReader read_to_end buffer handling #125
Conversation
WalkthroughAdjusted TeeReader’s Read::read_to_end implementation so it writes only the bytes appended during the read. It now records the initial buffer length, delegates to read_to_end, and writes the slice of newly added data to the internal writer. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller
participant T as TeeReader
participant R as Inner Reader
participant W as Internal Writer
C->>T: read_to_end(buf)
Note over T: Record original_len = buf.len()
T->>R: read_to_end(buf)
R-->>T: n = bytes appended (or error)
alt n > 0
T->>W: write_all(buf[original_len..])
W-->>T: ok
T-->>C: return n
else n == 0
T-->>C: return 0
end
opt error from R
T-->>C: propagate error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
io-utils/src/lib.rs (3)
43-44: Clarify the doc comment to match behavior.Make it explicit that only newly appended bytes are written.
- /// Reads all remaining data into a buffer and writes it to the internal writer. + /// Reads all remaining data into `buf` and writes only the newly appended bytes to the internal writer.
79-94: Add a test for pre-filled buffers to prevent regressions.Covers the case where
bufalready has data and ensures only appended bytes are tee’d.@@ fn tee_read_to_end() { @@ assert_eq!(&written[..n], b"stream this"); } + + #[test] + fn tee_read_to_end_prefilled_buf_writes_only_appended() { + let input_data = b"stream this"; + let input = Cursor::new(input_data); + let output = Cursor::new(Vec::new()); + + let mut tee = input.tee(output); + let mut buf = b"prefix-".to_vec(); + + let n = tee.read_to_end(&mut buf).unwrap(); + assert_eq!(n, input_data.len()); + assert_eq!(buf, b"prefix-stream this"); + + let (_input, output) = tee.into_inner(); + let written = output.into_inner(); + assert_eq!(written, b"stream this"); + }
44-49: Optional: write partial data on error.If
read_to_endappends some bytes then errors, you currently return early and write nothing. If you want “best effort” teeing, write the appended slice before propagating the error.- let n = self.reader.read_to_end(buf)?; - self.writer.write_all(&buf[len..])?; - Ok(n) + match self.reader.read_to_end(buf) { + Ok(n) => { + self.writer.write_all(&buf[len..])?; + Ok(n) + } + Err(e) => { + if buf.len() > len { + // Best-effort: ignore a secondary write error to preserve original read error. + let _ = self.writer.write_all(&buf[len..]); + } + Err(e) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
io-utils/src/lib.rs(1 hunks)
⏰ 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). (8)
- GitHub Check: Check
- GitHub Check: Shared / Static Analysis (intents-contract, contract, near, defuse, 1, ., 1.18, false, --filter-paths "nod...
- GitHub Check: Shared / Static Analysis (intents-poa-factory, contract, near, poa-factory, 1, ., 1.18, false, --filter-pa...
- GitHub Check: Shared / Static Analysis (intents-poa-token, contract, near, poa-token, 1, ., 1.18, false, --filter-paths ...
- GitHub Check: Shared security analysis / Static Analysis (intents-poa-factory, contract, near, poa-factory, 1, ., 1.18, false, --filter-pa...
- GitHub Check: Shared security analysis / Static Analysis (intents-poa-token, contract, near, poa-token, 1, ., 1.18, false, --filter-paths ...
- GitHub Check: Shared security analysis / Static Analysis (intents-contract, contract, near, defuse, 1, ., 1.18, false, --filter-paths "nod...
- GitHub Check: Security Audit - report
🔇 Additional comments (1)
io-utils/src/lib.rs (1)
45-49: Correct fix: write only newly appended bytes in read_to_end.Recording the initial len and writing
&buf[len..]matches Read::read_to_end semantics and fixes the previous bug. Return valuenremains correct.
Summary
TeeReader::read_to_endwrites only newly read bytes to the tee'd writerTesting
cargo test -p defuse-io-utilshttps://chatgpt.com/codex/tasks/task_b_68b9fa7b5288832c8b15a178fb27d15c
Summary by CodeRabbit