You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "codeAnqiang-ma (via GitHub)" <gi...@apache.org> on 2026/08/13 11:53:24 UTC

[PR] fix(core): Respect logical length in Buffer vectored views [opendal]

codeAnqiang-ma opened a new pull request, #8064:
URL: https://github.com/apache/opendal/pull/8064

   # Which issue does this PR close?
   
   Closes #8063.
   
   # Rationale for this change
   
   For a non-contiguous `Buffer`, `to_io_slice()` and the `bytes::Buf::chunks_vectored()` impl destructure `Inner::NonContiguous` with `..`, dropping the logical `size` field, and then walk each part to its **physical** end. Because `truncate` / `slice` / `split_to` / `split_off` shrink `size` without dropping `parts`, both methods then hand back more bytes than `remaining()` — for a 2-byte view of `["abc", "def"]` they expose all 6 bytes.
   
   That violates the documented `bytes::Buf::chunks_vectored` contract ("the sum of the lengths of all the buffers written to `dst` will be less than or equal to `Buf::remaining()`"), and it contradicts `chunk()` and `Iterator::next` in the same type, which both clamp with `.min(*size)` to honor the `Inner::NonContiguous` invariant ("the logic view … spans `size` bytes").
   
   #4481 already fixed the `offset` half of this problem in these same two methods; the `size` clamp was not added then.
   
   # What changes are included in this PR?
   
   Both non-contiguous branches now bind `size`, track it as a running `remaining`, clamp each slice with `.min(remaining)`, and stop once it reaches zero — mirroring what `chunk()` already does. Nothing else changes.
   
   Regression tests `test_vectored_views_after_slice`, `test_vectored_views_after_split_to` and `test_vectored_views_after_split_off` assert that both vectored views yield exactly the logical content. They fail on `main` and pass here.
   
   <details>
   <summary>Test evidence</summary>
   
   Before the fix (tests applied to unmodified `main`), all three fail — the vectored views expose the whole physical buffer:
   
   ```
   ---- types::buffer::tests::test_vectored_views_after_slice stdout ----
   assertion failed: `(left == right)`
    (
   <    b"abcdef",
   <    b"abcdef",
   >    b"ab",
   >    b"ab",
    )
   
   failures:
       types::buffer::tests::test_vectored_views_after_slice
       types::buffer::tests::test_vectored_views_after_split_off
       types::buffer::tests::test_vectored_views_after_split_to
   
   test result: FAILED. 0 passed; 3 failed
   ```
   
   After the fix:
   
   ```
   $ cargo test -p opendal-core --lib --features "tokio/time"
   test result: ok. 182 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
   
   $ cargo fmt --all -- --check                                              # clean
   $ cargo clippy -p opendal-core --all-targets --features "tokio/time" -- -D warnings   # clean
   ```
   
   `--features "tokio/time"` is only needed to compile `opendal-core`'s `#[cfg(test)]` code when the package is tested in isolation; it is unrelated to this change. The full-workspace `--all-features` clippy/nextest gate and the behavior tests were not run locally.
   
   </details>
   
   # Are there any user-facing changes?
   
   Yes — a bug fix; no signature or API change. A shortened non-contiguous `Buffer` passed to a vectored write now yields only its logical bytes instead of also emitting the truncated tail.
   
   To be precise about the blast radius: there is **no active data corruption in-tree**. The only in-tree vectored write backend, `compfs`, takes its bytes via `Buffer::by_ref().collect()` — the `Iterator` impl, which already respects `size` — so it never reached this path. The defect matters at the public API and trait-contract level: `to_io_slice` is `pub` and documented for vectored writes, and `chunks_vectored` is a `bytes::Buf` impl, so downstream users and any future in-tree consumer doing a vectored write over a shortened view are affected.
   
   # AI Usage Statement
   
   Prepared with AI assistance: Cursor, using Anthropic Claude Opus 5. The AI drafted the patch, the regression tests and this description; I reproduced the defect locally, verified the tests fail before the change and pass after it, and reviewed every line of the diff.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] fix(core): Respect logical length in Buffer vectored views [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #8064:
URL: https://github.com/apache/opendal/pull/8064


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org