Make panicking Path/PathBuf conversions fallible

To avoid unintended panics and to make it clear which conversions can
fail and which cannot, this patch changes all panicking Path/PathBuf
conversions to return a Result instead.

Fixes: #63

CHANGELOG.md

@@ -25,6 +25,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Enforced const evaluation for `path!`.
 - Removed `cstr_core` and `cty` dependencies.
 - Updated `littlefs2-sys` dependency to 0.2.0.
+- Replace all panicking `Path`/`PathBuf` conversions with fallible alternatives:
+  - Return a `Result` from `Path::from_str_with_nul`.
+  - Replace the `From<_>` implementations for `Path` and `PathBuf` with `TryFrom<_>`, except for `From<&Path> for PathBuf`.
 
 ### Removed
 

src/fs.rs

@@ -1254,7 +1254,7 @@ impl<'a, Storage: driver::Storage> Filesystem<'a, Storage> {
         let path_slice = path.as_ref().as_bytes();
         for i in 0..path_slice.len() {
             if path_slice[i] == b'/' {
-                let dir = PathBuf::from(&path_slice[..i]);
+                let dir = PathBuf::try_from(&path_slice[..i])?;
                 #[cfg(test)]
                 println!("generated PathBuf dir {:?} using i = {}", &dir, i);
                 match self.create_dir(&dir) {
@@ -1362,6 +1362,7 @@ impl<'a, Storage: driver::Storage> Filesystem<'a, Storage> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::path;
     use core::convert::TryInto;
     use driver::Storage as LfsStorage;
     use io::Result as LfsResult;
@@ -1483,7 +1484,7 @@ mod tests {
                 // (...)
                 // fs.remove_dir_all(&PathBuf::from(b"/tmp\0"))?;
                 // fs.remove_dir_all(&PathBuf::from(b"/tmp"))?;
-                fs.remove_dir_all(&PathBuf::from("/tmp"))?;
+                fs.remove_dir_all(path!("/tmp"))?;
             }
 
             Ok(())
@@ -1493,7 +1494,7 @@ mod tests {
         let mut alloc = Allocation::new();
         let fs = Filesystem::mount(&mut alloc, &mut test_storage).unwrap();
         // fs.write(b"/z.txt\0".try_into().unwrap(), &jackson5).unwrap();
-        fs.write(&PathBuf::from("z.txt"), jackson5).unwrap();
+        fs.write(path!("z.txt"), jackson5).unwrap();
     }
 
     #[cfg(feature = "dir-entry-path")]

src/lib.rs

@@ -93,7 +93,7 @@ Separately, keeping track of the allocations is a chore, we hope that
 ```
 # use littlefs2::fs::{Filesystem, File, OpenOptions};
 # use littlefs2::io::prelude::*;
-# use littlefs2::path::PathBuf;
+# use littlefs2::path;
 #
 # use littlefs2::{consts, ram_storage, driver, io::Result};
 #
@@ -113,7 +113,7 @@ let mut fs = Filesystem::mount(&mut alloc, &mut storage).unwrap();
 let mut buf = [0u8; 11];
 fs.open_file_with_options_and_then(
     |options| options.read(true).write(true).create(true),
-    &PathBuf::from(b"example.txt"),
+    path!("example.txt"),
     |file| {
         file.write(b"Why is black smoke coming out?!")?;
         file.seek(SeekFrom::End(-24)).unwrap();
@@ -199,7 +199,10 @@ pub struct Version {
 macro_rules! path {
     ($path:literal) => {{
         const _PATH: &$crate::path::Path =
-            $crate::path::Path::from_str_with_nul(::core::concat!($path, "\0"));
+            match $crate::path::Path::from_str_with_nul(::core::concat!($path, "\0")) {
+                Ok(path) => path,
+                Err(_) => panic!("invalid littlefs2 path"),
+            };
         _PATH
     }};
 }

src/path.rs

@@ -9,7 +9,7 @@ use core::{
     ops, ptr, slice, str,
 };
 
-use crate::consts;
+use crate::{consts, path};
 
 /// A path
 ///
@@ -94,14 +94,14 @@ impl<'a> Iterator for Ancestors<'a> {
             return None;
         } else if self.path == "/" {
             self.path = "";
-            return Some("/".into());
+            return Some(path!("/").into());
         }
 
         let item = self.path;
 
         let Some((rem, item_name)) = self.path.rsplit_once('/') else {
             self.path = "";
-            return Some(item.into());
+            return item.try_into().ok();
         };
 
         if self.path.starts_with('/') && rem.is_empty() {
@@ -114,7 +114,7 @@ impl<'a> Iterator for Ancestors<'a> {
         if item_name.is_empty() {
             self.next();
         }
-        Some(item.into())
+        item.try_into().ok()
     }
 }
 
@@ -135,17 +135,17 @@ impl<'a> Iterator for Iter<'a> {
         }
         if self.path.starts_with('/') {
             self.path = &self.path[1..];
-            return Some("/".into());
+            return Some(path!("/").into());
         }
 
         let Some((path, rem)) = self.path.split_once('/') else {
-            let ret_val = Some(self.path.into());
+            let ret_val = self.path.try_into().ok();
             self.path = "";
             return ret_val;
         };
 
         self.path = rem;
-        Some(path.into())
+        path.try_into().ok()
     }
 }
 
@@ -236,21 +236,9 @@ impl Path {
     ///
     /// The string must only consist of ASCII characters.  The last character must be null.  It
     /// must contain at most [`PATH_MAX`](`consts::PATH_MAX`) bytes, not including the trailing
-    /// null.  If these conditions are not met, this function panics.
-    pub const fn from_str_with_nul(s: &str) -> &Self {
-        let bytes = s.as_bytes();
-        let mut i = 0;
-        while i < bytes.len().saturating_sub(1) {
-            assert!(bytes[i] != 0, "must not contain null");
-            assert!(bytes[i].is_ascii(), "must be ASCII");
-            i += 1;
-        }
-        assert!(!bytes.is_empty(), "must not be empty");
-        assert!(bytes[i] == 0, "last byte must be null");
-        unsafe {
-            let cstr = CStr::from_bytes_with_nul_unchecked(bytes);
-            Self::from_cstr_unchecked(cstr)
-        }
+    /// null.  If these conditions are not met, this function returns an error.
+    pub const fn from_str_with_nul(s: &str) -> Result<&Self> {
+        Self::from_bytes_with_nul(s.as_bytes())
     }
 
     /// Creates a path from a byte buffer.
@@ -322,14 +310,16 @@ impl Path {
     pub fn parent(&self) -> Option<PathBuf> {
         let rk_path_bytes = self.as_ref()[..].as_bytes();
         match rk_path_bytes.iter().rposition(|x| *x == b'/') {
-            Some(0) if rk_path_bytes.len() != 1 => Some(PathBuf::from("/")),
+            Some(0) if rk_path_bytes.len() != 1 => Some(path!("/").into()),
             Some(slash_index) => {
                 // if we have a directory that ends with `/`,
                 // still need to "go up" one parent
                 if slash_index + 1 == rk_path_bytes.len() {
-                    PathBuf::from(&rk_path_bytes[..slash_index]).parent()
+                    PathBuf::try_from(&rk_path_bytes[..slash_index])
+                        .ok()?
+                        .parent()
                 } else {
-                    Some(PathBuf::from(&rk_path_bytes[..slash_index]))
+                    PathBuf::try_from(&rk_path_bytes[..slash_index]).ok()
                 }
             }
             None => None,
@@ -382,9 +372,11 @@ macro_rules! array_impls {
                 }
             }
 
-            impl From<&[u8; $N]> for PathBuf {
-                fn from(bytes: &[u8; $N]) -> Self {
-                    Self::from(&bytes[..])
+            impl TryFrom<&[u8; $N]> for PathBuf {
+                type Error = Error;
+
+                fn try_from(bytes: &[u8; $N]) -> Result<Self> {
+                    Self::try_from(&bytes[..])
                 }
             }
 
@@ -521,33 +513,43 @@ impl From<&Path> for PathBuf {
     }
 }
 
-impl From<&[u8]> for PathBuf {
-    /// Accepts byte string, with or without trailing nul.
-    ///
-    /// PANICS: when there are embedded nuls
-    fn from(bytes: &[u8]) -> Self {
+/// Accepts byte strings, with or without trailing nul.
+impl TryFrom<&[u8]> for PathBuf {
+    type Error = Error;
+
+    fn try_from(bytes: &[u8]) -> Result<Self> {
         // NB: This needs to set the final NUL byte, unless it already has one
         // It also checks that there are no inner NUL bytes
         let bytes = if !bytes.is_empty() && bytes[bytes.len() - 1] == b'\0' {
             &bytes[..bytes.len() - 1]
         } else {
             bytes
         };
-        let has_no_embedded_nul = !bytes.iter().any(|&byte| byte == b'\0');
-        assert!(has_no_embedded_nul);
+        if bytes.len() > consts::PATH_MAX {
+            return Err(Error::TooLarge);
+        }
+        for byte in bytes {
+            if *byte == 0 {
+                return Err(Error::NotCStr);
+            }
+            if !byte.is_ascii() {
+                return Err(Error::NotAscii);
+            }
+        }
 
         let mut buf = [0; consts::PATH_MAX_PLUS_ONE];
         let len = bytes.len();
-        assert!(len <= consts::PATH_MAX);
-        assert!(bytes.is_ascii());
         unsafe { ptr::copy_nonoverlapping(bytes.as_ptr(), buf.as_mut_ptr().cast(), len) }
-        Self { buf, len: len + 1 }
+        Ok(Self { buf, len: len + 1 })
     }
 }
 
-impl From<&str> for PathBuf {
-    fn from(s: &str) -> Self {
-        PathBuf::from(s.as_bytes())
+/// Accepts strings, with or without trailing nul.
+impl TryFrom<&str> for PathBuf {
+    type Error = Error;
+
+    fn try_from(s: &str) -> Result<Self> {
+        PathBuf::try_from(s.as_bytes())
     }
 }
 
@@ -597,7 +599,7 @@ impl<'de> serde::Deserialize<'de> for PathBuf {
                 if v.len() > consts::PATH_MAX {
                     return Err(E::invalid_length(v.len(), &self));
                 }
-                Ok(PathBuf::from(v))
+                PathBuf::try_from(v).map_err(|_| E::custom("invalid path buffer"))
             }
         }
 
@@ -670,24 +672,22 @@ mod tests {
 
     #[test]
     fn path_macro() {
-        assert_eq!(EMPTY, &*PathBuf::from(""));
-        assert_eq!(SLASH, &*PathBuf::from("/"));
+        assert_eq!(EMPTY, &*PathBuf::try_from("").unwrap());
+        assert_eq!(SLASH, &*PathBuf::try_from("/").unwrap());
     }
 
     // does not compile:
     // const NON_ASCII: &Path = path!("über");
     // const NULL: &Path = path!("ub\0er");
 
     #[test]
-    #[should_panic]
     fn nul_in_from_str_with_nul() {
-        Path::from_str_with_nul("ub\0er");
+        assert!(Path::from_str_with_nul("ub\0er").is_err());
     }
 
     #[test]
-    #[should_panic]
     fn non_ascii_in_from_str_with_nul() {
-        Path::from_str_with_nul("über");
+        assert!(Path::from_str_with_nul("über").is_err());
     }
 
     #[test]
@@ -725,7 +725,10 @@ mod tests {
 
     #[test]
     fn trailing_nuls() {
-        assert_eq!(PathBuf::from("abc"), PathBuf::from("abc\0"));
+        assert_eq!(
+            PathBuf::try_from("abc").unwrap(),
+            PathBuf::try_from("abc\0").unwrap()
+        );
     }
 
     #[test]

src/tests.rs

@@ -195,20 +195,20 @@ fn test_create() {
         assert_eq!(fs.available_blocks().unwrap(), 512 - 2);
         assert_eq!(fs.available_space().unwrap(), 130_560);
 
-        assert!(!crate::path::PathBuf::from(b"/test_open.txt").exists(fs));
+        assert!(!path!("/test_open.txt").exists(fs));
         assert_eq!(
             File::open_and_then(fs, b"/test_open.txt\0".try_into().unwrap(), |_| { Ok(()) })
                 .map(drop)
                 .unwrap_err(), // "real" contains_err is experimental
             Error::NoSuchEntry
         );
-        assert!(!crate::path::PathBuf::from(b"/test_open.txt").exists(fs));
+        assert!(!path!("/test_open.txt").exists(fs));
 
         fs.create_dir(b"/tmp\0".try_into().unwrap()).unwrap();
         assert_eq!(fs.available_blocks().unwrap(), 512 - 2 - 2);
 
         // can create new files
-        assert!(!crate::path::PathBuf::from(b"/tmp/test_open.txt").exists(fs));
+        assert!(!path!("/tmp/test_open.txt").exists(fs));
         fs.create_file_and_then(b"/tmp/test_open.txt\0".try_into().unwrap(), |file| {
             // can write to files
             assert!(file.write(&[0u8, 1, 2]).unwrap() == 3);
@@ -219,7 +219,7 @@ fn test_create() {
             // file.close()?;
             Ok(())
         })?;
-        assert!(crate::path::PathBuf::from(b"/tmp/test_open.txt").exists(fs));
+        assert!(path!("/tmp/test_open.txt").exists(fs));
 
         // // cannot remove non-empty directories
         assert_eq!(