From 496818ccd79e9bc093552887c923168defb13c6c Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Mon, 8 Jun 2020 18:31:45 +0200 Subject: [PATCH 1/7] Add methods to go from a nul-terminated Vec to a CString, checked and unchecked. Doc tests have been written and the documentation on the error type updated too. --- src/libstd/ffi/c_str.rs | 80 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 4bac9a4917d8f..f3a935ccc1130 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -234,15 +234,18 @@ pub struct NulError(usize, Vec); /// An error indicating that a nul byte was not in the expected position. /// -/// The slice used to create a [`CStr`] must have one and only one nul -/// byte at the end of the slice. +/// The slice used to create a [`CStr`] or the vector used to create a +/// [`CString`] must have one and only one nul byte, positioned at the end. /// /// This error is created by the /// [`from_bytes_with_nul`][`CStr::from_bytes_with_nul`] method on -/// [`CStr`]. See its documentation for more. +/// [`CStr`] or the [`from_vec_with_nul`][`CString::from_vec_with_nul`] method +/// on [`CString`]. See their documentation for more. /// /// [`CStr`]: struct.CStr.html /// [`CStr::from_bytes_with_nul`]: struct.CStr.html#method.from_bytes_with_nul +/// [`CString`]: struct.CString.html +/// [`CString::from_vec_with_nul`]: struct.CString.html#method.from_vec_with_nul /// /// # Examples /// @@ -632,6 +635,77 @@ impl CString { let this = mem::ManuallyDrop::new(self); unsafe { ptr::read(&this.inner) } } + + /// Converts a `Vec` of `u8` to a `CString` without checking the invariants + /// on the given `Vec`. + /// + /// # Safety + /// + /// The given `Vec` **must** have one nul byte as its last element. + /// This means it cannot be empty nor have any other nul byte anywhere else. + /// + /// # Example + /// + /// ``` + /// use std::ffi::CString; + /// assert_eq!( + /// unsafe { CString::from_vec_with_nul_unchecked(b"abc\0".to_vec()) }, + /// unsafe { CString::from_vec_unchecked(b"abc".to_vec()) } + /// ); + /// ``` + #[stable(feature = "cstring_from_vec_with_nul", since = "1.46.0")] + pub unsafe fn from_vec_with_nul_unchecked(v: Vec) -> Self { + Self { inner: v.into_boxed_slice() } + } + + /// Attempts to converts a `Vec` of `u8` to a `CString`. + /// + /// Runtime checks are present to ensure there is only one nul byte in the + /// `Vec`, its last element. + /// + /// # Errors + /// + /// If a nul byte is present and not the last element or no nul bytes + /// is present, an error will be returned. + /// + /// # Examples + /// + /// A successful conversion will produce the same result as [`new`] when + /// called without the ending nul byte. + /// + /// ``` + /// use std::ffi::CString; + /// assert_eq!( + /// CString::from_vec_with_nul(b"abc\0".to_vec()) + /// .expect("CString::from_vec_with_nul failed"), + /// CString::new(b"abc".to_vec()) + /// ); + /// ``` + /// + /// A incorrectly formatted vector will produce an error. + /// + /// ``` + /// use std::ffi::{CString, FromBytesWithNulError}; + /// // Interior nul byte + /// let _: FromBytesWithNulError = CString::from_vec_with_nul(b"a\0bc".to_vec()).unwrap_err(); + /// // No nul byte + /// let _: FromBytesWithNulError = CString::from_vec_with_nul(b"abc".to_vec()).unwrap_err(); + /// ``` + /// + /// [`new`]: #method.new + #[stable(feature = "cstring_from_vec_with_nul", since = "1.46.0")] + pub fn from_vec_with_nul(v: Vec) -> Result { + let nul_pos = memchr::memchr(0, &v); + match nul_pos { + Some(nul_pos) if nul_pos + 1 == v.len() => { + // SAFETY: We know there is only one nul byte, at the end + // of the vec. + Ok(unsafe { Self::from_vec_with_nul_unchecked(v) }) + } + Some(nul_pos) => Err(FromBytesWithNulError::interior_nul(nul_pos)), + None => Err(FromBytesWithNulError::not_nul_terminated()), + } + } } // Turns this `CString` into an empty string to prevent From b03164e6679284b984437fe82092682cf7c984f8 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Tue, 9 Jun 2020 22:15:05 +0200 Subject: [PATCH 2/7] Move to unstable, linking the issue --- src/libstd/ffi/c_str.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index f3a935ccc1130..6f7dc091897f4 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -653,7 +653,7 @@ impl CString { /// unsafe { CString::from_vec_unchecked(b"abc".to_vec()) } /// ); /// ``` - #[stable(feature = "cstring_from_vec_with_nul", since = "1.46.0")] + #[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] pub unsafe fn from_vec_with_nul_unchecked(v: Vec) -> Self { Self { inner: v.into_boxed_slice() } } @@ -693,7 +693,7 @@ impl CString { /// ``` /// /// [`new`]: #method.new - #[stable(feature = "cstring_from_vec_with_nul", since = "1.46.0")] + #[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] pub fn from_vec_with_nul(v: Vec) -> Result { let nul_pos = memchr::memchr(0, &v); match nul_pos { From 7f3bb398fa90d68c737dd7e00a3813e0620ba472 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Wed, 10 Jun 2020 23:13:45 +0200 Subject: [PATCH 3/7] Add a TryFrom> impl that mirror from_vec_with_nul --- src/libstd/ffi/c_str.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 6f7dc091897f4..3a3b51fd353b2 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -1,6 +1,7 @@ use crate::ascii; use crate::borrow::{Borrow, Cow}; use crate::cmp::Ordering; +use crate::convert::TryFrom; use crate::error::Error; use crate::fmt::{self, Write}; use crate::io; @@ -853,6 +854,19 @@ impl From> for CString { } } +#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] +impl TryFrom> for CString { + type Error = FromBytesWithNulError; + + /// See the document about [`from_vec_with_nul`] for more + /// informations about the behaviour of this method. + /// + /// [`from_vec_with_nul`]: struct.CString.html#method.from_vec_with_nul + fn try_from(value: Vec) -> Result { + Self::from_vec_with_nul(value) + } +} + #[stable(feature = "more_box_slice_clone", since = "1.29.0")] impl Clone for Box { #[inline] From 6b955268d71907d9049a399a0e0a33f6448e1221 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 16:55:03 +0200 Subject: [PATCH 4/7] Fix the link in the TryFrom impl --- src/libstd/ffi/c_str.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 3a3b51fd353b2..298f6c33457d8 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -861,7 +861,7 @@ impl TryFrom> for CString { /// See the document about [`from_vec_with_nul`] for more /// informations about the behaviour of this method. /// - /// [`from_vec_with_nul`]: struct.CString.html#method.from_vec_with_nul + /// [`from_vec_with_nul`]: CString::from_vec_with_nul fn try_from(value: Vec) -> Result { Self::from_vec_with_nul(value) } From 5f4eb27a0dc5d17e4a74f1a2d71a8f5a30916e3a Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 14 Jun 2020 19:21:44 +0200 Subject: [PATCH 5/7] Removing the TryFrom impl --- src/libstd/ffi/c_str.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 298f6c33457d8..6f7dc091897f4 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -1,7 +1,6 @@ use crate::ascii; use crate::borrow::{Borrow, Cow}; use crate::cmp::Ordering; -use crate::convert::TryFrom; use crate::error::Error; use crate::fmt::{self, Write}; use crate::io; @@ -854,19 +853,6 @@ impl From> for CString { } } -#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] -impl TryFrom> for CString { - type Error = FromBytesWithNulError; - - /// See the document about [`from_vec_with_nul`] for more - /// informations about the behaviour of this method. - /// - /// [`from_vec_with_nul`]: CString::from_vec_with_nul - fn try_from(value: Vec) -> Result { - Self::from_vec_with_nul(value) - } -} - #[stable(feature = "more_box_slice_clone", since = "1.29.0")] impl Clone for Box { #[inline] From 685f06612dc8842b734181e380eae0f9b1a9483f Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 14 Jun 2020 23:21:40 +0200 Subject: [PATCH 6/7] Add a new error type for the new method --- src/libstd/ffi/c_str.rs | 96 +++++++++++++++++++++++++++++++++++++++++ src/libstd/ffi/mod.rs | 2 + 2 files changed, 98 insertions(+) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 6f7dc091897f4..956100bca6a55 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -260,6 +260,32 @@ pub struct FromBytesWithNulError { kind: FromBytesWithNulErrorKind, } +/// An error indicating that a nul byte was not in the expected position. +/// +/// The vector used to create a [`CString`] must have one and only one nul byte, +/// positioned at the end. +/// +/// This error is created by the [`from_vec_with_nul`] method on [`CString`]. +/// See its documentation for more. +/// +/// [`CString`]: struct.CString.html +/// [`from_vec_with_nul`]: struct.CString.html#method.from_vec_with_nul +/// +/// # Examples +/// +/// ``` +/// #![feature(cstring_from_vec_with_nul)] +/// use std::ffi::{CString, FromVecWithNulError}; +/// +/// let _: FromVecWithNulError = CString::from_vec_with_nul(b"f\0oo".to_vec()).unwrap_err(); +/// ``` +#[derive(Clone, PartialEq, Eq, Debug)] +#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] +pub struct FromVecWithNulError { + error_kind: FromBytesWithNulErrorKind, + bytes: Vec, +} + #[derive(Clone, PartialEq, Eq, Debug)] enum FromBytesWithNulErrorKind { InteriorNul(usize), @@ -275,6 +301,59 @@ impl FromBytesWithNulError { } } +#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] +impl FromVecWithNulError { + /// Returns a slice of [`u8`]s bytes that were attempted to convert to a [`CString`]. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// #![feature(cstring_from_vec_with_nul)] + /// use std::ffi::CString; + /// + /// // Some invalid bytes in a vector + /// let bytes = b"f\0oo".to_vec(); + /// + /// let value = CString::from_vec_with_nul(bytes.clone()); + /// + /// assert_eq!(&bytes[..], value.unwrap_err().as_bytes()); + /// ``` + /// + /// [`CString`]: struct.CString.html + pub fn as_bytes(&self) -> &[u8] { + &self.bytes[..] + } + + /// Returns the bytes that were attempted to convert to a [`CString`]. + /// + /// This method is carefully constructed to avoid allocation. It will + /// consume the error, moving out the bytes, so that a copy of the bytes + /// does not need to be made. + /// + /// # Examples + /// + /// Basic usage: + /// + /// ``` + /// #![feature(cstring_from_vec_with_nul)] + /// use std::ffi::CString; + /// + /// // Some invalid bytes in a vector + /// let bytes = b"f\0oo".to_vec(); + /// + /// let value = CString::from_vec_with_nul(bytes.clone()); + /// + /// assert_eq!(bytes, value.unwrap_err().into_bytes()); + /// ``` + /// + /// [`CString`]: struct.CString.html + pub fn into_bytes(self) -> Vec { + self.bytes + } +} + /// An error indicating invalid UTF-8 when converting a [`CString`] into a [`String`]. /// /// `CString` is just a wrapper over a buffer of bytes with a nul @@ -1039,6 +1118,23 @@ impl fmt::Display for FromBytesWithNulError { } } +#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] +impl Error for FromVecWithNulError {} + +#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] +impl fmt::Display for FromVecWithNulError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.error_kind { + FromBytesWithNulErrorKind::InteriorNul(pos) => { + write!(f, "data provided contains an interior nul byte at pos {}", pos) + } + FromBytesWithNulErrorKind::NotNulTerminated => { + write!(f, "data provided is not nul terminated") + } + } + } +} + impl IntoStringError { /// Consumes this error, returning original [`CString`] which generated the /// error. diff --git a/src/libstd/ffi/mod.rs b/src/libstd/ffi/mod.rs index 5aca7b7476a52..f442d7fde1a5e 100644 --- a/src/libstd/ffi/mod.rs +++ b/src/libstd/ffi/mod.rs @@ -157,6 +157,8 @@ #[stable(feature = "cstr_from_bytes", since = "1.10.0")] pub use self::c_str::FromBytesWithNulError; +#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] +pub use self::c_str::FromVecWithNulError; #[stable(feature = "rust1", since = "1.0.0")] pub use self::c_str::{CStr, CString, IntoStringError, NulError}; From 47cc5cca7e64a434fa15680b7dcc621ca6f1abbf Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sun, 14 Jun 2020 23:22:36 +0200 Subject: [PATCH 7/7] Update to use the new error type and correctly compile the doc tests --- src/libstd/ffi/c_str.rs | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 956100bca6a55..3537a9f8656a2 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -234,18 +234,14 @@ pub struct NulError(usize, Vec); /// An error indicating that a nul byte was not in the expected position. /// -/// The slice used to create a [`CStr`] or the vector used to create a -/// [`CString`] must have one and only one nul byte, positioned at the end. +/// The slice used to create a [`CStr`] must have one and only one nul byte, +/// positioned at the end. /// -/// This error is created by the -/// [`from_bytes_with_nul`][`CStr::from_bytes_with_nul`] method on -/// [`CStr`] or the [`from_vec_with_nul`][`CString::from_vec_with_nul`] method -/// on [`CString`]. See their documentation for more. +/// This error is created by the [`from_bytes_with_nul`] method on [`CStr`]. +/// See its documentation for more. /// /// [`CStr`]: struct.CStr.html -/// [`CStr::from_bytes_with_nul`]: struct.CStr.html#method.from_bytes_with_nul -/// [`CString`]: struct.CString.html -/// [`CString::from_vec_with_nul`]: struct.CString.html#method.from_vec_with_nul +/// [`from_bytes_with_nul`]: struct.CStr.html#method.from_bytes_with_nul /// /// # Examples /// @@ -726,6 +722,7 @@ impl CString { /// # Example /// /// ``` + /// #![feature(cstring_from_vec_with_nul)] /// use std::ffi::CString; /// assert_eq!( /// unsafe { CString::from_vec_with_nul_unchecked(b"abc\0".to_vec()) }, @@ -753,27 +750,29 @@ impl CString { /// called without the ending nul byte. /// /// ``` + /// #![feature(cstring_from_vec_with_nul)] /// use std::ffi::CString; /// assert_eq!( /// CString::from_vec_with_nul(b"abc\0".to_vec()) /// .expect("CString::from_vec_with_nul failed"), - /// CString::new(b"abc".to_vec()) + /// CString::new(b"abc".to_vec()).expect("CString::new failed") /// ); /// ``` /// /// A incorrectly formatted vector will produce an error. /// /// ``` - /// use std::ffi::{CString, FromBytesWithNulError}; + /// #![feature(cstring_from_vec_with_nul)] + /// use std::ffi::{CString, FromVecWithNulError}; /// // Interior nul byte - /// let _: FromBytesWithNulError = CString::from_vec_with_nul(b"a\0bc".to_vec()).unwrap_err(); + /// let _: FromVecWithNulError = CString::from_vec_with_nul(b"a\0bc".to_vec()).unwrap_err(); /// // No nul byte - /// let _: FromBytesWithNulError = CString::from_vec_with_nul(b"abc".to_vec()).unwrap_err(); + /// let _: FromVecWithNulError = CString::from_vec_with_nul(b"abc".to_vec()).unwrap_err(); /// ``` /// /// [`new`]: #method.new #[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")] - pub fn from_vec_with_nul(v: Vec) -> Result { + pub fn from_vec_with_nul(v: Vec) -> Result { let nul_pos = memchr::memchr(0, &v); match nul_pos { Some(nul_pos) if nul_pos + 1 == v.len() => { @@ -781,8 +780,14 @@ impl CString { // of the vec. Ok(unsafe { Self::from_vec_with_nul_unchecked(v) }) } - Some(nul_pos) => Err(FromBytesWithNulError::interior_nul(nul_pos)), - None => Err(FromBytesWithNulError::not_nul_terminated()), + Some(nul_pos) => Err(FromVecWithNulError { + error_kind: FromBytesWithNulErrorKind::InteriorNul(nul_pos), + bytes: v, + }), + None => Err(FromVecWithNulError { + error_kind: FromBytesWithNulErrorKind::NotNulTerminated, + bytes: v, + }), } } }