tokio-rs / bytes

Utilities for working with bytes
MIT License
1.89k stars 284 forks source link

Add optional support for the arbitrary crate #715

Closed morr0ne closed 2 months ago

morr0ne commented 4 months ago

This PRs adds optional support for the arbitrary crate by deriving the Arbitrary trait for both Bytes and BytesMut. This is incredibly useful for fuzz testing as it allows for structure aware fuzzing. Adding this traits directly allows downstream users to easily derive the traits for their structs:

E.g. from one of my projects

#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Arbitrary)]
pub struct Message {
    pub object_id: u32,
    pub opcode: u16,
    pub payload: Bytes,
}

Without bytes implementing Arbitrary directly this would require some kinda of wrapper struct.

seanmonstar commented 4 months ago

Thanks for the PR! However, I wouldn't want to add this public dependency to bytes.

morr0ne commented 4 months ago

Thanks for the PR! However, I wouldn't want to add this public dependency to bytes.

What's the issue with having arbitrary as a depedency? It is optional and not enabled by default just like serde

morr0ne commented 2 months ago

Could you please provide an update on the status of this? The changes seem straightforward and ready for merging. Are there any specific concerns or reasons I am not aware of?

seanmonstar commented 2 months ago

There is no status update. My personal reason for not wanting to depend on the additional dependency is that it's another piece to maintain, and every new dependency is another potential case of it releasing a new breaking release (2.0), and we have tied our stability to publicly exposed version.

By comparison, anything that is holding some Bytes could implement Arbitrary in their own crate, even without direct support here. So, in my mind, the benefits don't outweigh the risks.