Fix Lua String soundness when borrowing &str or &[u8].

Make borrowing using new `BorrowedStr` and `BorrowedBytes` types that holds strong reference to Lua.

Author: khvzak

examples/async_http_server.rs

@@ -68,14 +68,14 @@ impl hyper::service::Service<Request<Incoming>> for Svc {
                     if let Some(headers) = lua_resp.get::<_, Option<Table>>("headers")? {
                         for pair in headers.pairs::<String, LuaString>() {
                             let (h, v) = pair?;
-                            resp = resp.header(&h, v.as_bytes());
+                            resp = resp.header(&h, &*v.as_bytes());
                         }
                     }
 
                     // Set body
                     let body = lua_resp
                         .get::<_, Option<LuaString>>("body")?
-                        .map(|b| Full::new(Bytes::copy_from_slice(b.as_bytes())).boxed())
+                        .map(|b| Full::new(Bytes::copy_from_slice(&b.as_bytes())).boxed())
                         .unwrap_or_else(|| Empty::<Bytes>::new().boxed());
 
                     Ok(resp.body(body).unwrap())

src/conversion.rs

@@ -464,7 +464,7 @@ impl FromLua for CString {
                 message: Some("expected string or number".to_string()),
             })?;
 
-        match CStr::from_bytes_with_nul(string.as_bytes_with_nul()) {
+        match CStr::from_bytes_with_nul(&string.as_bytes_with_nul()) {
             Ok(s) => Ok(s.into()),
             Err(_) => Err(Error::FromLuaConversionError {
                 from: ty,
@@ -500,7 +500,7 @@ impl FromLua for BString {
     fn from_lua(value: Value, lua: &Lua) -> Result<Self> {
         let ty = value.type_name();
         match value {
-            Value::String(s) => Ok(s.as_bytes().into()),
+            Value::String(s) => Ok((*s.as_bytes()).into()),
             #[cfg(feature = "luau")]
             Value::UserData(ud) if ud.1 == crate::types::SubtypeId::Buffer => unsafe {
                 let lua = ud.0.lua.lock();
@@ -509,15 +509,15 @@ impl FromLua for BString {
                 mlua_assert!(!buf.is_null(), "invalid Luau buffer");
                 Ok(slice::from_raw_parts(buf as *const u8, size).into())
             },
-            _ => Ok(lua
+            _ => Ok((*lua
                 .coerce_string(value)?
                 .ok_or_else(|| Error::FromLuaConversionError {
                     from: ty,
                     to: "BString",
                     message: Some("expected string or number".to_string()),
                 })?
-                .as_bytes()
-                .into()),
+                .as_bytes())
+            .into()),
         }
     }
 

src/lib.rs

@@ -111,7 +111,7 @@ pub use crate::multi::Variadic;
 pub use crate::state::{GCMode, Lua, LuaOptions};
 // pub use crate::scope::Scope;
 pub use crate::stdlib::StdLib;
-pub use crate::string::String;
+pub use crate::string::{BorrowedBytes, BorrowedStr, String};
 pub use crate::table::{Table, TableExt, TablePairs, TableSequence};
 pub use crate::thread::{Thread, ThreadStatus};
 pub use crate::types::{AppDataRef, AppDataRefMut, Integer, LightUserData, Number, RegistryKey};

src/serde/de.rs

@@ -135,8 +135,8 @@ impl<'de> serde::Deserializer<'de> for Deserializer {
             #[cfg(feature = "luau")]
             Value::Vector(_) => self.deserialize_seq(visitor),
             Value::String(s) => match s.to_str() {
-                Ok(s) => visitor.visit_str(s),
-                Err(_) => visitor.visit_bytes(s.as_bytes()),
+                Ok(s) => visitor.visit_str(&s),
+                Err(_) => visitor.visit_bytes(&s.as_bytes()),
             },
             Value::Table(ref t) if t.raw_len() > 0 || t.is_array() => self.deserialize_seq(visitor),
             Value::Table(_) => self.deserialize_map(visitor),

src/string.rs

@@ -1,8 +1,9 @@
-use std::borrow::{Borrow, Cow};
+use std::borrow::Borrow;
 use std::hash::{Hash, Hasher};
+use std::ops::Deref;
 use std::os::raw::c_void;
 use std::string::String as StdString;
-use std::{fmt, slice, str};
+use std::{cmp, fmt, slice, str};
 
 #[cfg(feature = "serialize")]
 use {
@@ -11,6 +12,7 @@ use {
 };
 
 use crate::error::{Error, Result};
+use crate::state::LuaGuard;
 use crate::types::ValueRef;
 
 /// Handle to an internal Lua string.
@@ -20,7 +22,7 @@ use crate::types::ValueRef;
 pub struct String(pub(crate) ValueRef);
 
 impl String {
-    /// Get a `&str` slice if the Lua string is valid UTF-8.
+    /// Get a [`BorrowedStr`] if the Lua string is valid UTF-8.
     ///
     /// # Examples
     ///
@@ -39,15 +41,17 @@ impl String {
     /// # }
     /// ```
     #[inline]
-    pub fn to_str(&self) -> Result<&str> {
-        str::from_utf8(self.as_bytes()).map_err(|e| Error::FromLuaConversionError {
+    pub fn to_str(&self) -> Result<BorrowedStr> {
+        let BorrowedBytes(bytes, guard) = self.as_bytes();
+        let s = str::from_utf8(bytes).map_err(|e| Error::FromLuaConversionError {
             from: "string",
             to: "&str",
             message: Some(e.to_string()),
-        })
+        })?;
+        Ok(BorrowedStr(s, guard))
     }
 
-    /// Converts this string to a [`Cow<str>`].
+    /// Converts this string to a [`StdString`].
     ///
     /// Any non-Unicode sequences are replaced with [`U+FFFD REPLACEMENT CHARACTER`][U+FFFD].
     ///
@@ -66,8 +70,8 @@ impl String {
     /// # }
     /// ```
     #[inline]
-    pub fn to_string_lossy(&self) -> Cow<'_, str> {
-        StdString::from_utf8_lossy(self.as_bytes())
+    pub fn to_string_lossy(&self) -> StdString {
+        StdString::from_utf8_lossy(&self.as_bytes()).into_owned()
     }
 
     /// Get the bytes that make up this string.
@@ -88,13 +92,18 @@ impl String {
     /// # }
     /// ```
     #[inline]
-    pub fn as_bytes(&self) -> &[u8] {
-        let nulled = self.as_bytes_with_nul();
-        &nulled[..nulled.len() - 1]
+    pub fn as_bytes(&self) -> BorrowedBytes {
+        let (bytes, guard) = unsafe { self.to_slice() };
+        BorrowedBytes(&bytes[..bytes.len() - 1], guard)
     }
 
     /// Get the bytes that make up this string, including the trailing nul byte.
-    pub fn as_bytes_with_nul(&self) -> &[u8] {
+    pub fn as_bytes_with_nul(&self) -> BorrowedBytes {
+        let (bytes, guard) = unsafe { self.to_slice() };
+        BorrowedBytes(bytes, guard)
+    }
+
+    unsafe fn to_slice(&self) -> (&[u8], LuaGuard) {
         let lua = self.0.lua.lock();
         let ref_thread = lua.ref_thread();
         unsafe {
@@ -108,7 +117,7 @@ impl String {
             // string type
             let data = ffi::lua_tolstring(ref_thread, self.0.index, &mut size);
 
-            slice::from_raw_parts(data as *const u8, size + 1)
+            (slice::from_raw_parts(data as *const u8, size + 1), lua)
         }
     }
 
@@ -127,7 +136,7 @@ impl fmt::Debug for String {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         let bytes = self.as_bytes();
         // Check if the string is valid utf8
-        if let Ok(s) = str::from_utf8(bytes) {
+        if let Ok(s) = str::from_utf8(&bytes) {
             return s.fmt(f);
         }
 
@@ -152,22 +161,9 @@ impl fmt::Debug for String {
     }
 }
 
-impl AsRef<[u8]> for String {
-    fn as_ref(&self) -> &[u8] {
-        self.as_bytes()
-    }
-}
-
-impl Borrow<[u8]> for String {
-    fn borrow(&self) -> &[u8] {
-        self.as_bytes()
-    }
-}
-
 // Lua strings are basically &[u8] slices, so implement PartialEq for anything resembling that.
 //
-// This makes our `String` comparable with `Vec<u8>`, `[u8]`, `&str`, `String` and `mlua::String`
-// itself.
+// This makes our `String` comparable with `Vec<u8>`, `[u8]`, `&str` and `String`.
 //
 // The only downside is that this disallows a comparison with `Cow<str>`, as that only implements
 // `AsRef<str>`, which collides with this impl. Requiring `AsRef<str>` would fix that, but limit us
@@ -181,6 +177,18 @@ where
     }
 }
 
+impl PartialEq<String> for String {
+    fn eq(&self, other: &String) -> bool {
+        self.as_bytes() == other.as_bytes()
+    }
+}
+
+impl PartialEq<&String> for String {
+    fn eq(&self, other: &&String) -> bool {
+        self.as_bytes() == other.as_bytes()
+    }
+}
+
 impl Eq for String {}
 
 impl Hash for String {
@@ -196,12 +204,127 @@ impl Serialize for String {
         S: Serializer,
     {
         match self.to_str() {
-            Ok(s) => serializer.serialize_str(s),
-            Err(_) => serializer.serialize_bytes(self.as_bytes()),
+            Ok(s) => serializer.serialize_str(&s),
+            Err(_) => serializer.serialize_bytes(&self.as_bytes()),
         }
     }
 }
 
+/// A borrowed string (`&str`) that holds a strong reference to the Lua state.
+pub struct BorrowedStr<'a>(&'a str, #[allow(unused)] LuaGuard);
+
+impl Deref for BorrowedStr<'_> {
+    type Target = str;
+
+    #[inline(always)]
+    fn deref(&self) -> &str {
+        self.0
+    }
+}
+
+impl Borrow<str> for BorrowedStr<'_> {
+    #[inline(always)]
+    fn borrow(&self) -> &str {
+        self.0
+    }
+}
+
+impl AsRef<str> for BorrowedStr<'_> {
+    #[inline(always)]
+    fn as_ref(&self) -> &str {
+        self.0
+    }
+}
+
+impl fmt::Display for BorrowedStr<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl fmt::Debug for BorrowedStr<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl<T> PartialEq<T> for BorrowedStr<'_>
+where
+    T: AsRef<str>,
+{
+    fn eq(&self, other: &T) -> bool {
+        self.0 == other.as_ref()
+    }
+}
+
+impl<T> PartialOrd<T> for BorrowedStr<'_>
+where
+    T: AsRef<str>,
+{
+    fn partial_cmp(&self, other: &T) -> Option<cmp::Ordering> {
+        self.0.partial_cmp(other.as_ref())
+    }
+}
+
+/// A borrowed byte slice (`&[u8]`) that holds a strong reference to the Lua state.
+pub struct BorrowedBytes<'a>(&'a [u8], #[allow(unused)] LuaGuard);
+
+impl Deref for BorrowedBytes<'_> {
+    type Target = [u8];
+
+    #[inline(always)]
+    fn deref(&self) -> &[u8] {
+        self.0
+    }
+}
+
+impl Borrow<[u8]> for BorrowedBytes<'_> {
+    #[inline(always)]
+    fn borrow(&self) -> &[u8] {
+        self.0
+    }
+}
+
+impl AsRef<[u8]> for BorrowedBytes<'_> {
+    #[inline(always)]
+    fn as_ref(&self) -> &[u8] {
+        self.0
+    }
+}
+
+impl fmt::Debug for BorrowedBytes<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl<T> PartialEq<T> for BorrowedBytes<'_>
+where
+    T: AsRef<[u8]>,
+{
+    fn eq(&self, other: &T) -> bool {
+        self.0 == other.as_ref()
+    }
+}
+
+impl<T> PartialOrd<T> for BorrowedBytes<'_>
+where
+    T: AsRef<[u8]>,
+{
+    fn partial_cmp(&self, other: &T) -> Option<cmp::Ordering> {
+        self.0.partial_cmp(other.as_ref())
+    }
+}
+
+impl<'a> IntoIterator for BorrowedBytes<'a> {
+    type Item = &'a u8;
+    type IntoIter = slice::Iter<'a, u8>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.into_iter()
+    }
+}
+
 #[cfg(test)]
 mod assertions {
     use super::*;

src/value.rs

@@ -1,4 +1,3 @@
-use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::collections::{vec_deque, HashSet, VecDeque};
 use std::ops::{Deref, DerefMut};
@@ -12,7 +11,7 @@ use num_traits::FromPrimitive;
 use crate::error::{Error, Result};
 use crate::function::Function;
 use crate::state::{Lua, RawLua};
-use crate::string::String;
+use crate::string::{BorrowedStr, String};
 use crate::table::Table;
 use crate::thread::Thread;
 use crate::types::{Integer, LightUserData, Number, SubtypeId};
@@ -330,19 +329,20 @@ impl Value {
         }
     }
 
-    /// Cast the value to [`str`].
+    /// Cast the value to [`BorrowedStr`].
     ///
-    /// If the value is a Lua [`String`], try to convert it to [`str`] or return `None` otherwise.
+    /// If the value is a Lua [`String`], try to convert it to [`BorrowedStr`] or return `None`
+    /// otherwise.
     #[inline]
-    pub fn as_str(&self) -> Option<&str> {
+    pub fn as_str(&self) -> Option<BorrowedStr> {
         self.as_string().and_then(|s| s.to_str().ok())
     }
 
-    /// Cast the value to [`Cow<str>`].
+    /// Cast the value to [`StdString`].
     ///
-    /// If the value is a Lua [`String`], converts it to [`Cow<str>`] or returns `None` otherwise.
+    /// If the value is a Lua [`String`], converts it to [`StdString`] or returns `None` otherwise.
     #[inline]
-    pub fn as_string_lossy(&self) -> Option<Cow<str>> {
+    pub fn as_string_lossy(&self) -> Option<StdString> {
         self.as_string().map(|s| s.to_string_lossy())
     }
 
@@ -478,7 +478,7 @@ impl Value {
             (Value::Integer(_) | Value::Number(_), _) => Ordering::Less,
             (_, Value::Integer(_) | Value::Number(_)) => Ordering::Greater,
             // String
-            (Value::String(a), Value::String(b)) => a.as_bytes().cmp(b.as_bytes()),
+            (Value::String(a), Value::String(b)) => a.as_bytes().cmp(&b.as_bytes()),
             (Value::String(_), _) => Ordering::Less,
             (_, Value::String(_)) => Ordering::Greater,
             // Other variants can be randomly ordered