Panic in BytesMut::split_to when out of bounds (#252) (#253)

CHANGELOG.md

@@ -1,3 +1,9 @@
+# 0.4.12 (March 6, 2019)
+
+### Added
+- Implement `FromIterator<&'a u8>` for `BytesMut`/`Bytes` (#244).
+- Implement `Buf` for `VecDeque` (#249).
+
 # 0.4.11 (November 17, 2018)
 
 * Use raw pointers for potentially racy loads (#233).

Cargo.toml

@@ -6,11 +6,11 @@ name          = "bytes"
 # - Update CHANGELOG.md.
 # - Update doc URL.
 # - Create "v0.4.x" git tag.
-version       = "0.4.11"
+version       = "0.4.12"
 license       = "MIT"
 authors       = ["Carl Lerche <[email protected]>"]
 description   = "Types and traits for working with bytes"
-documentation = "https://docs.rs/bytes/0.4.11/bytes"
+documentation = "https://docs.rs/bytes/0.4.12/bytes"
 homepage      = "https://github.com/carllerche/bytes"
 repository    = "https://github.com/carllerche/bytes"
 readme        = "README.md"

README.md

@@ -5,7 +5,7 @@ A utility library for working with bytes.
 [![Crates.io](https://img.shields.io/crates/v/bytes.svg?maxAge=2592000)](https://crates.io/crates/bytes)
 [![Build Status](https://travis-ci.org/carllerche/bytes.svg?branch=master)](https://travis-ci.org/carllerche/bytes)
 
-[Documentation](https://carllerche.github.io/bytes/bytes/index.html)
+[Documentation](https://docs.rs/bytes/0.4.12/bytes/)
 
 ## Usage
 
@@ -13,7 +13,7 @@ To use `bytes`, first add this to your `Cargo.toml`:
 
 ```toml
 [dependencies]
-bytes = "0.4"
+bytes = "0.4.12"
 ```
 
 Next, add this to your crate:
@@ -30,7 +30,7 @@ Serde support is optional and disabled by default. To enable use the feature `se
 
 ```toml
 [dependencies]
-bytes = { version = "0.4", features = ["serde"] }
+bytes = { version = "0.4.12", features = ["serde"] }
 ```
 
 ## License

src/buf/mod.rs

@@ -24,6 +24,7 @@ mod into_buf;
 mod iter;
 mod reader;
 mod take;
+mod vec_deque;
 mod writer;
 
 pub use self::buf::Buf;

src/buf/vec_deque.rs

@@ -0,0 +1,39 @@
+use std::collections::VecDeque;
+
+use super::Buf;
+
+impl Buf for VecDeque<u8> {
+    fn remaining(&self) -> usize {
+        self.len()
+    }
+
+    fn bytes(&self) -> &[u8] {
+        let (s1, s2) = self.as_slices();
+        if s1.is_empty() {
+            s2
+        } else {
+            s1
+        }
+    }
+
+    fn advance(&mut self, cnt: usize) {
+        self.drain(..cnt);
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn hello_world() {
+        let mut buffer: VecDeque<u8> = VecDeque::new();
+        buffer.extend(b"hello world");
+        assert_eq!(11, buffer.remaining());
+        assert_eq!(b"hello world", buffer.bytes());
+        buffer.advance(6);
+        assert_eq!(b"world", buffer.bytes());
+        buffer.extend(b" piece");
+        assert_eq!(b"world piece" as &[u8], &buffer.collect::<Vec<u8>>()[..]);
+    }
+}

src/bytes.rs

@@ -273,7 +273,7 @@ pub struct BytesMut {
 // The rest of `arc`'s bytes are used as part of the inline buffer, which means
 // that those bytes need to be located next to the `ptr`, `len`, and `cap`
 // fields, which make up the rest of the inline buffer. This requires special
-// casing the layout of `Inner` depending on if the target platform is bit or
+// casing the layout of `Inner` depending on if the target platform is big or
 // little endian.
 //
 // On little endian platforms, the `arc` field must be the first field in the
@@ -926,6 +926,18 @@ impl FromIterator<u8> for Bytes {
     }
 }
 
+impl<'a> FromIterator<&'a u8> for BytesMut {
+    fn from_iter<T: IntoIterator<Item = &'a u8>>(into_iter: T) -> Self {
+        BytesMut::from_iter(into_iter.into_iter().map(|b| *b))
+    }
+}
+
+impl<'a> FromIterator<&'a u8> for Bytes {
+    fn from_iter<T: IntoIterator<Item = &'a u8>>(into_iter: T) -> Self {
+        BytesMut::from_iter(into_iter).freeze()
+    }
+}
+
 impl PartialEq for Bytes {
     fn eq(&self, other: &Bytes) -> bool {
         self.inner.as_ref() == other.inner.as_ref()
@@ -1253,6 +1265,8 @@ impl BytesMut {
     ///
     /// Panics if `at > len`.
     pub fn split_to(&mut self, at: usize) -> BytesMut {
+        assert!(at <= self.len());
+
         BytesMut {
             inner: self.inner.split_to(at),
         }
@@ -2429,6 +2443,10 @@ impl Inner {
         // bits, so even without any explicit atomic operations, reading the
         // flag will be correct.
         //
+        // This is undefind behavior due to a data race, but experimental
+        // evidence shows that it works in practice (discussion:
+        // https://internals.rust-lang.org/t/bit-wise-reasoning-for-atomic-accesses/8853).
+        //
         // This function is very critical performance wise as it is called for
         // every operation. Performing an atomic load would mess with the
         // compiler's ability to optimize. Simple benchmarks show up to a 10%
@@ -2463,7 +2481,7 @@ impl Inner {
         // function.
         let prev = unsafe {
             let p: &AtomicPtr<Shared> = &self.arc;
-            let p: &usize = mem::transmute(p);
+            let p: *const usize = mem::transmute(p);
             *p
         };
 
@@ -2570,35 +2588,51 @@ fn original_capacity_from_repr(repr: usize) -> usize {
 
 #[test]
 fn test_original_capacity_to_repr() {
-    for &cap in &[0, 1, 16, 1000] {
-        assert_eq!(0, original_capacity_to_repr(cap));
-    }
+    assert_eq!(original_capacity_to_repr(0), 0);
 
-    for &cap in &[1024, 1025, 1100, 2000, 2047] {
-        assert_eq!(1, original_capacity_to_repr(cap));
-    }
+    let max_width = 32;
 
-    for &cap in &[2048, 2049] {
-        assert_eq!(2, original_capacity_to_repr(cap));
-    }
+    for width in 1..(max_width + 1) {
+        let cap = 1 << width - 1;
 
-    // TODO: more
+        let expected = if width < MIN_ORIGINAL_CAPACITY_WIDTH {
+            0
+        } else if width < MAX_ORIGINAL_CAPACITY_WIDTH {
+            width - MIN_ORIGINAL_CAPACITY_WIDTH
+        } else {
+            MAX_ORIGINAL_CAPACITY_WIDTH - MIN_ORIGINAL_CAPACITY_WIDTH
+        };
 
-    for &cap in &[65536, 65537, 68000, 1 << 17, 1 << 18, 1 << 20, 1 << 30] {
-        assert_eq!(7, original_capacity_to_repr(cap), "cap={}", cap);
+        assert_eq!(original_capacity_to_repr(cap), expected);
+
+        if width > 1 {
+            assert_eq!(original_capacity_to_repr(cap + 1), expected);
+        }
+
+        //  MIN_ORIGINAL_CAPACITY_WIDTH must be bigger than 7 to pass tests below
+        if width == MIN_ORIGINAL_CAPACITY_WIDTH + 1 {
+            assert_eq!(original_capacity_to_repr(cap - 24), expected - 1);
+            assert_eq!(original_capacity_to_repr(cap + 76), expected);
+        } else if width == MIN_ORIGINAL_CAPACITY_WIDTH + 2 {
+            assert_eq!(original_capacity_to_repr(cap - 1), expected - 1);
+            assert_eq!(original_capacity_to_repr(cap - 48), expected - 1);
+        }
     }
 }
 
 #[test]
 fn test_original_capacity_from_repr() {
     assert_eq!(0, original_capacity_from_repr(0));
-    assert_eq!(1024, original_capacity_from_repr(1));
-    assert_eq!(1024 * 2, original_capacity_from_repr(2));
-    assert_eq!(1024 * 4, original_capacity_from_repr(3));
-    assert_eq!(1024 * 8, original_capacity_from_repr(4));
-    assert_eq!(1024 * 16, original_capacity_from_repr(5));
-    assert_eq!(1024 * 32, original_capacity_from_repr(6));
-    assert_eq!(1024 * 64, original_capacity_from_repr(7));
+
+    let min_cap = 1 << MIN_ORIGINAL_CAPACITY_WIDTH;
+
+    assert_eq!(min_cap, original_capacity_from_repr(1));
+    assert_eq!(min_cap * 2, original_capacity_from_repr(2));
+    assert_eq!(min_cap * 4, original_capacity_from_repr(3));
+    assert_eq!(min_cap * 8, original_capacity_from_repr(4));
+    assert_eq!(min_cap * 16, original_capacity_from_repr(5));
+    assert_eq!(min_cap * 32, original_capacity_from_repr(6));
+    assert_eq!(min_cap * 64, original_capacity_from_repr(7));
 }
 
 unsafe impl Send for Inner {}

src/lib.rs

@@ -69,7 +69,7 @@
 //! and `BufMut` are infallible.
 
 #![deny(warnings, missing_docs, missing_debug_implementations)]
-#![doc(html_root_url = "https://docs.rs/bytes/0.4.11")]
+#![doc(html_root_url = "https://docs.rs/bytes/0.4.12")]
 
 extern crate byteorder;
 extern crate iovec;

tests/test_bytes.rs

@@ -258,15 +258,10 @@ fn split_to_oob_mut() {
 }
 
 #[test]
+#[should_panic]
 fn split_to_uninitialized() {
     let mut bytes = BytesMut::with_capacity(1024);
-    let other = bytes.split_to(128);
-
-    assert_eq!(bytes.len(), 0);
-    assert_eq!(bytes.capacity(), 896);
-
-    assert_eq!(other.len(), 0);
-    assert_eq!(other.capacity(), 128);
+    let _other = bytes.split_to(128);
 }
 
 #[test]