fix(clap_lex): Deprecate unsound OsStrExt::split_at

clap-rs · Mar 28, 2023 · 06e2a33 · 06e2a33
1 parent 56dc953
commit 06e2a33
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 3 deletions.
diff --git a/clap_lex/src/ext.rs b/clap_lex/src/ext.rs
@@ -193,6 +193,7 @@ pub trait OsStrExt: private::Sealed {
     /// assert_eq!("Per", first);
     /// assert_eq!(" Martin-Löf", last);
     /// ```
+    #[deprecated(since = "4.1.0", note = "This is not sound for all `index`")]
     fn split_at(&self, index: usize) -> (&OsStr, &OsStr);
     /// Splits the string on the first occurrence of the specified delimiter and
     /// returns prefix before delimiter and suffix after delimiter.
@@ -251,7 +252,7 @@ impl OsStrExt for OsStr {
     }
 
     fn split_at(&self, index: usize) -> (&OsStr, &OsStr) {
-        // BUG: This is unsafe
+        // BUG: This is unsafe and has been deprecated
         unsafe {
             let bytes = to_bytes(self);
             let (first, second) = bytes.split_at(index);
@@ -341,3 +342,14 @@ impl<'s, 'n> Iterator for Split<'s, 'n> {
         }
     }
 }
+
+/// Split an `OsStr`
+///
+/// # Safety
+///
+/// `index` must be at a valid UTF-8 boundary
+pub(crate) unsafe fn split_at(os: &OsStr, index: usize) -> (&OsStr, &OsStr) {
+    let bytes = to_bytes(os);
+    let (first, second) = bytes.split_at(index);
+    (to_os_str(first), to_os_str(second))
+}
diff --git a/clap_lex/src/lib.rs b/clap_lex/src/lib.rs
@@ -433,7 +433,9 @@ impl<'s> ShortFlags<'s> {
         if let Some((index, _)) = self.utf8_prefix.next() {
             self.utf8_prefix = "".char_indices();
             self.invalid_suffix = None;
-            return Some(self.inner.split_at(index).1);
+            // SAFETY: `char_indices` ensures `index` is at a valid UTF-8 boundary
+            let remainder = unsafe { ext::split_at(self.inner, index).1 };
+            return Some(remainder);
         }
 
         if let Some(suffix) = self.invalid_suffix {
@@ -457,7 +459,8 @@ fn split_nonutf8_once(b: &OsStr) -> (&str, Option<&OsStr>) {
     match b.try_str() {
         Ok(s) => (s, None),
         Err(err) => {
-            let (valid, after_valid) = b.split_at(err.valid_up_to());
+            // SAFETY: `char_indices` ensures `index` is at a valid UTF-8 boundary
+            let (valid, after_valid) = unsafe { ext::split_at(b, err.valid_up_to()) };
             let valid = valid.try_str().unwrap();
             (valid, Some(after_valid))
         }