tokio-rs / bytes

Utilities for working with bytes
MIT License
1.87k stars 278 forks source link

`Bytes::slice` doesn't always return an actual slice of the original `Bytes` #557

Closed ilyvion closed 2 years ago

ilyvion commented 2 years ago

Due to this: https://github.com/tokio-rs/bytes/blob/38fd42acbaced11ff19f0a4ca2af44a308af5063/src/bytes.rs#L268-L270 the resulting Bytes has no relationship with the original, meaning that if you're using Bytes in a tool that slices and dices it quite a lot outside your direct control (like nom, which is what I'm using) including doing zero-length slices, and later asking for what the "offset" of a slice of the underlying data was, you get very wrong, completely bogus data.

In particular, the invariant from "regular slices," i.e.

let b = b"hello";
let zs = &b[0..0];

assert_eq!(b.as_ptr(), zs.as_ptr());

does not hold up for Bytes:

let b: Bytes = b"hello".as_ref().into();
let zs = b.slice(0..0);

assert_eq!(b.as_ref().as_ptr(), zs.as_ref().as_ptr());
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0x55c28cf4504d`,
 right: `0x55c28cf45070`', src/main.rs:7:5
ilyvion commented 2 years ago

Looks like this optimization was added in 43ac8e5494e9404327f971937edb092b8cae6a2b. I don't really need it reversed as long as I can get a zero-length true slice of a Bytes in some other way; though I haven't found an obvious way... I was hoping I could do the slicing in &[u8] and then call Bytes::slice_ref, but it has the same broken behavior: https://github.com/tokio-rs/bytes/blob/38fd42acbaced11ff19f0a4ca2af44a308af5063/src/bytes.rs#L308-L310

Darksonn commented 2 years ago

There's a good reason to do it like this. When the ref-count is equal to one, the allocation can be reused. This behavior ensures that empty slices don't prevent the allocation from being reused.

I recommend that you create a wrapper struct with a start and end index instead.

ilyvion commented 2 years ago

Thanks, a wrapper solution works for me. I'll close this now, thanks for the feedback. 👍