Make `UnsafeSlice::as_slice` unsafe to call

In an earlier commit we marked `as_slice` as safe to call, stating
that only construction is unsafe. However, that is incorrect. The
ctor of `UnsafeSlice` does nothing unsafe, unless we make as_slice
safe. However, since the type is not bounded to any lifetime,
making `as_slice` safe to call assumes a very "rarely true"
safety contract: that the pointers are valid throughout the
execution of the program, id est they are static.

However, that assumption is entirely incorrect for our use case,
hence I'm marking this as `unsafe` again.

server/src/protocol/iter.rs

@@ -44,6 +44,7 @@ pub struct BorrowedAnyArrayIter<'a> {
 
 impl<'a> Deref for BorrowedAnyArrayIter<'a> {
     type Target = Iter<'a, UnsafeSlice>;
+    #[inline(always)]
     fn deref(&self) -> &Self::Target {
         &self.iter
     }
@@ -55,38 +56,56 @@ impl<'a> AnyArrayIter<'a> {
     /// ## Safety
     /// - Valid source pointers
     /// - Source pointers exist as long as the iterator is used
+    #[inline(always)]
     pub const unsafe fn new(iter: Iter<'a, UnsafeSlice>) -> AnyArrayIter<'a> {
         Self { iter }
     }
     /// Check if the iter is empty
+    #[inline(always)]
     pub fn is_empty(&self) -> bool {
         ExactSizeIterator::len(self) == 0
     }
     /// Returns a borrowed iterator => simply put, advancing the returned iterator does not
     /// affect the base iterator owned by this object
+    #[inline(always)]
     pub fn as_ref(&'a self) -> BorrowedAnyArrayIter<'a> {
         BorrowedAnyArrayIter {
             iter: self.iter.as_ref().iter(),
         }
     }
     /// Returns the starting ptr of the `AnyArray`
+    #[inline(always)]
     pub unsafe fn as_ptr(&self) -> *const UnsafeSlice {
         self.iter.as_ref().as_ptr()
     }
     /// Returns the next value in uppercase
+    #[inline(always)]
     pub fn next_uppercase(&mut self) -> Option<Box<[u8]>> {
-        self.iter
-            .next()
-            .map(|v| v.as_slice().to_ascii_uppercase().into_boxed_slice())
+        self.iter.next().map(|v| {
+            unsafe {
+                // UNSAFE(@ohsayan): The ctor of `Self` allows us to "assume" this is safe
+                v.as_slice()
+            }
+            .to_ascii_uppercase()
+            .into_boxed_slice()
+        })
     }
+    #[inline(always)]
     pub fn next_lowercase(&mut self) -> Option<Box<[u8]>> {
-        self.iter
-            .next()
-            .map(|v| v.as_slice().to_ascii_lowercase().into_boxed_slice())
+        self.iter.next().map(|v| {
+            unsafe {
+                // UNSAFE(@ohsayan): The ctor of `Self` allows us to "assume" this is safe
+                v.as_slice()
+            }
+            .to_ascii_lowercase()
+            .into_boxed_slice()
+        })
     }
+    #[inline(always)]
     pub unsafe fn next_lowercase_unchecked(&mut self) -> Box<[u8]> {
         self.next_lowercase().unwrap_or_else(|| impossible!())
     }
+    #[inline(always)]
     pub unsafe fn next_uppercase_unchecked(&mut self) -> Box<[u8]> {
         match self.next_uppercase() {
             Some(s) => s,
@@ -95,6 +114,7 @@ impl<'a> AnyArrayIter<'a> {
             }
         }
     }
+    #[inline(always)]
     /// Returns the next value without any checks
     pub unsafe fn next_unchecked(&mut self) -> &'a [u8] {
         match self.next() {
@@ -102,16 +122,20 @@ impl<'a> AnyArrayIter<'a> {
             None => unreachable_unchecked(),
         }
     }
+    #[inline(always)]
     /// Returns the next value without any checks as an owned copy of [`Bytes`]
     pub unsafe fn next_unchecked_bytes(&mut self) -> Bytes {
         Bytes::copy_from_slice(self.next_unchecked())
     }
+    #[inline(always)]
     pub fn map_next<T>(&mut self, cls: fn(&[u8]) -> T) -> Option<T> {
         self.next().map(cls)
     }
+    #[inline(always)]
     pub fn next_string_owned(&mut self) -> Option<String> {
         self.map_next(|v| String::from_utf8_lossy(v).to_string())
     }
+    #[inline(always)]
     pub unsafe fn into_inner(self) -> Iter<'a, UnsafeSlice> {
         self.iter
     }
@@ -140,17 +164,26 @@ unsafe impl DerefUnsafeSlice for Bytes {
 
 impl<'a> Iterator for AnyArrayIter<'a> {
     type Item = &'a [u8];
+    #[inline(always)]
     fn next(&mut self) -> Option<Self::Item> {
-        self.iter.next().map(|v| v.as_slice())
+        self.iter.next().map(|v| unsafe {
+            // UNSAFE(@ohsayan): The ctor of `Self` allows us to "assume" this is safe
+            v.as_slice()
+        })
     }
+    #[inline(always)]
     fn size_hint(&self) -> (usize, Option<usize>) {
         self.iter.size_hint()
     }
 }
 
 impl<'a> DoubleEndedIterator for AnyArrayIter<'a> {
+    #[inline(always)]
     fn next_back(&mut self) -> Option<<Self as Iterator>::Item> {
-        self.iter.next_back().map(|v| v.as_slice())
+        self.iter.next_back().map(|v| unsafe {
+            // UNSAFE(@ohsayan): The ctor of `Self` allows us to "assume" this is safe
+            v.as_slice()
+        })
     }
 }
 
@@ -159,14 +192,22 @@ impl<'a> FusedIterator for AnyArrayIter<'a> {}
 
 impl<'a> Iterator for BorrowedAnyArrayIter<'a> {
     type Item = &'a [u8];
+    #[inline(always)]
     fn next(&mut self) -> Option<Self::Item> {
-        self.iter.next().map(|v| v.as_slice())
+        self.iter.next().map(|v| unsafe {
+            // UNSAFE(@ohsayan): The ctor of `AnyArrayIter` allows us to "assume" this is safe
+            v.as_slice()
+        })
     }
 }
 
 impl<'a> DoubleEndedIterator for BorrowedAnyArrayIter<'a> {
+    #[inline(always)]
     fn next_back(&mut self) -> Option<<Self as Iterator>::Item> {
-        self.iter.next_back().map(|v| v.as_slice())
+        self.iter.next_back().map(|v| unsafe {
+            // UNSAFE(@ohsayan): The ctor of `AnyArrayIter` allows us to "assume" this is safe
+            v.as_slice()
+        })
     }
 }
 

server/src/protocol/mod.rs

@@ -77,17 +77,20 @@ impl fmt::Debug for UnsafeSlice {
 
 impl UnsafeSlice {
     /// Create a new `UnsafeSlice`
-    pub const unsafe fn new(start_ptr: *const u8, len: usize) -> Self {
+    #[inline(always)]
+    pub const fn new(start_ptr: *const u8, len: usize) -> Self {
         Self { start_ptr, len }
     }
     /// Return self as a slice
-    pub fn as_slice(&self) -> &[u8] {
-        unsafe {
-            // UNSAFE(@ohsayan): Just like core::slice, we resemble the same idea:
-            // we assume that the unsafe construction was correct and hence *assume*
-            // that calling this is safe
-            slice::from_raw_parts(self.start_ptr, self.len)
-        }
+    /// ## Safety
+    /// The caller must ensure that the pointer and length used when constructing the slice
+    /// are valid when this is called
+    #[inline(always)]
+    pub unsafe fn as_slice(&self) -> &[u8] {
+        // UNSAFE(@ohsayan): Just like core::slice, we resemble the same idea:
+        // we assume that the unsafe construction was correct and hence *assume*
+        // that calling this is safe
+        slice::from_raw_parts(self.start_ptr, self.len)
     }
 }
 
@@ -129,12 +132,17 @@ impl SimpleQuery {
     #[cfg(test)]
     fn into_owned(self) -> OwnedSimpleQuery {
         OwnedSimpleQuery {
-            data: self.data.iter().map(|v| v.as_slice().to_owned()).collect(),
+            data: self
+                .data
+                .iter()
+                .map(|v| unsafe { v.as_slice() }.to_owned())
+                .collect(),
         }
     }
     pub const fn new(data: HeapArray<UnsafeSlice>) -> Self {
         Self { data }
     }
+    #[inline(always)]
     pub fn as_slice(&self) -> &[UnsafeSlice] {
         &self.data
     }
@@ -166,7 +174,11 @@ impl PipelinedQuery {
             data: self
                 .data
                 .iter()
-                .map(|v| v.iter().map(|v| v.as_slice().to_owned()).collect())
+                .map(|v| {
+                    v.iter()
+                        .map(|v| unsafe { v.as_slice() }.to_owned())
+                        .collect()
+                })
                 .collect(),
         }
     }

server/src/protocol/raw_parser.rs

@@ -151,7 +151,7 @@ pub(super) trait RawParserExt: RawParser + RawParserMeta {
     /// Attempt to read an `usize` from the buffer
     fn read_usize(&mut self) -> ParseResult<usize> {
         let line = self.read_line_pedantic()?;
-        let bytes = line.as_slice();
+        let bytes = unsafe { line.as_slice() };
         let mut ret = 0usize;
         for byte in bytes {
             if byte.is_ascii_digit() {

server/src/protocol/v1/tests.rs

@@ -41,7 +41,7 @@ fn parse_simple_query() {
     let q: Vec<String> = if let Query::Simple(q) = q {
         q.as_slice()
             .iter()
-            .map(|v| String::from_utf8_lossy(v.as_slice()).to_string())
+            .map(|v| String::from_utf8_lossy(unsafe { v.as_slice() }).to_string())
             .collect()
     } else {
         panic!("Expected simple query")
@@ -67,7 +67,7 @@ fn parse_pipelined_query() {
             .iter()
             .map(|sq| {
                 sq.iter()
-                    .map(|v| String::from_utf8_lossy(v.as_slice()).to_string())
+                    .map(|v| String::from_utf8_lossy(unsafe { v.as_slice() }).to_string())
                     .collect()
             })
             .collect()

server/src/protocol/v2/tests.rs

@@ -70,7 +70,7 @@ fn get_slices(slices: &[&[u8]]) -> Packets {
 
 fn ensure_zero_reads(parser: &mut Parser) {
     let r = parser.read_until(0).unwrap();
-    let slice = r.as_slice();
+    let slice = unsafe { r.as_slice() };
     assert_eq!(slice, b"");
     assert!(slice.is_empty());
 }
@@ -318,7 +318,7 @@ fn read_until_nonempty() {
         ensure_zero_reads(&mut parser);
         // now read the entire length; should always work
         let r = parser.read_until(len).unwrap();
-        let slice = r.as_slice();
+        let slice = unsafe { r.as_slice() };
         assert_eq!(slice, src.as_slice());
         assert_eq!(slice.len(), len);
         // even after the buffer is exhausted, `0` should always work
@@ -346,7 +346,7 @@ fn read_until_more_bytes() {
     let sample1 = v!(b"abcd1");
     let mut p1 = Parser::new(&sample1);
     assert_eq!(
-        p1.read_until(&sample1.len() - 1).unwrap().as_slice(),
+        unsafe { p1.read_until(&sample1.len() - 1).unwrap().as_slice() },
         &sample1[..&sample1.len() - 1]
     );
     // ensure we have not exhasuted
@@ -354,7 +354,10 @@ fn read_until_more_bytes() {
     ensure_remaining(&p1, 1);
     let sample2 = v!(b"abcd1234567890!@#$");
     let mut p2 = Parser::new(&sample2);
-    assert_eq!(p2.read_until(4).unwrap().as_slice(), &sample2[..4]);
+    assert_eq!(
+        unsafe { p2.read_until(4).unwrap().as_slice() },
+        &sample2[..4]
+    );
     // ensure we have not exhasuted
     ensure_not_exhausted(&p2);
     ensure_remaining(&p2, sample2.len() - 4);
@@ -366,7 +369,7 @@ fn read_line_special_case_only_lf() {
     let b = v!(b"\n");
     let mut parser = Parser::new(&b);
     let r = parser.read_line().unwrap();
-    let slice = r.as_slice();
+    let slice = unsafe { r.as_slice() };
     assert_eq!(slice, b"");
     assert!(slice.is_empty());
     // ensure it is exhausted
@@ -383,7 +386,7 @@ fn read_line() {
         } else {
             // should work
             assert_eq!(
-                parser.read_line().unwrap().as_slice(),
+                unsafe { parser.read_line().unwrap().as_slice() },
                 &src.as_slice()[..len - 1]
             );
             // now, we attempt to read which should work
@@ -405,7 +408,7 @@ fn read_line_more_bytes() {
     let sample1 = v!(b"abcd\n1");
     let mut p1 = Parser::new(&sample1);
     let line = p1.read_line().unwrap();
-    assert_eq!(line.as_slice(), b"abcd");
+    assert_eq!(unsafe { line.as_slice() }, b"abcd");
     // we should still have one remaining
     ensure_not_exhausted(&p1);
     ensure_remaining(&p1, 1);
@@ -416,13 +419,13 @@ fn read_line_subsequent_lf() {
     let sample1 = v!(b"abcd\n1\n");
     let mut p1 = Parser::new(&sample1);
     let line = p1.read_line().unwrap();
-    assert_eq!(line.as_slice(), b"abcd");
+    assert_eq!(unsafe { line.as_slice() }, b"abcd");
     // we should still have two octets remaining
     ensure_not_exhausted(&p1);
     ensure_remaining(&p1, 2);
     // and we should be able to read in another line
     let line = p1.read_line().unwrap();
-    assert_eq!(line.as_slice(), b"1");
+    assert_eq!(unsafe { line.as_slice() }, b"1");
     ensure_exhausted(&p1);
 }
 
@@ -439,7 +442,7 @@ fn read_line_pedantic_okay() {
         } else {
             // should work
             assert_eq!(
-                parser.read_line_pedantic().unwrap().as_slice(),
+                unsafe { parser.read_line_pedantic().unwrap().as_slice() },
                 &src.as_slice()[..len - 1]
             );
             // now, we attempt to read which should work