From 15ec95a98de3e135b89c81a369f6013004e89cae Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 9 Nov 2022 22:28:55 +0000 Subject: [PATCH 1/3] Make Buf::as_str private and unsafe, add safety docs serde::de::format::Buf is a private type, so this makes it explicit by declaring the type `pub(super)`. In addition, it marks the function `Buf::as_str` as unsafe, which lets us document the callsites with `// Safety: ...` comments to explain why it is safe to use. --- serde/src/de/format.rs | 8 ++++---- serde/src/de/mod.rs | 12 ++++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/serde/src/de/format.rs b/serde/src/de/format.rs index 58ec0968d..aa4d53f98 100644 --- a/serde/src/de/format.rs +++ b/serde/src/de/format.rs @@ -1,19 +1,19 @@ use lib::fmt::{self, Write}; use lib::str; -pub struct Buf<'a> { +pub(super) struct Buf<'a> { bytes: &'a mut [u8], offset: usize, } impl<'a> Buf<'a> { - pub fn new(bytes: &'a mut [u8]) -> Self { + pub(super) fn new(bytes: &'a mut [u8]) -> Self { Buf { bytes, offset: 0 } } - pub fn as_str(&self) -> &str { + pub(super) unsafe fn as_str(&self) -> &str { let slice = &self.bytes[..self.offset]; - unsafe { str::from_utf8_unchecked(slice) } + str::from_utf8_unchecked(slice) } } diff --git a/serde/src/de/mod.rs b/serde/src/de/mod.rs index d9dafbe1e..43084a486 100644 --- a/serde/src/de/mod.rs +++ b/serde/src/de/mod.rs @@ -1376,7 +1376,11 @@ pub trait Visitor<'de>: Sized { let mut buf = [0u8; 58]; let mut writer = format::Buf::new(&mut buf); fmt::Write::write_fmt(&mut writer, format_args!("integer `{}` as i128", v)).unwrap(); - Err(Error::invalid_type(Unexpected::Other(writer.as_str()), &self)) + + // Safety: This is safe because we only wrote UTF-8 into the buffer. + let s = unsafe { writer.as_str() }; + + Err(Error::invalid_type(Unexpected::Other(s), &self)) } } @@ -1438,7 +1442,11 @@ pub trait Visitor<'de>: Sized { let mut buf = [0u8; 57]; let mut writer = format::Buf::new(&mut buf); fmt::Write::write_fmt(&mut writer, format_args!("integer `{}` as u128", v)).unwrap(); - Err(Error::invalid_type(Unexpected::Other(writer.as_str()), &self)) + + // Safety: This is safe because we only wrote UTF-8 into the buffer. + let s = unsafe { writer.as_str() }; + + Err(Error::invalid_type(Unexpected::Other(s), &self)) } } From 1050f6b80851ba2bbef25992da2646bd79b04882 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Wed, 9 Nov 2022 22:41:28 +0000 Subject: [PATCH 2/3] Change comment to // Safety: ... This changes a comment to be explicit on how it's safe we can avoid validating UTF-8. --- serde/src/ser/impls.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index 8e8655582..ccef4d291 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -736,8 +736,9 @@ impl Serialize for net::Ipv4Addr { // Skip over delimiters that we initialized buf with written += format_u8(*oct, &mut buf[written + 1..]) + 1; } - // We've only written ASCII bytes to the buffer, so it is valid UTF-8 - serializer.serialize_str(unsafe { str::from_utf8_unchecked(&buf[..written]) }) + // Safety: We've only written ASCII bytes to the buffer, so it is valid UTF-8 + let buf = unsafe { str::from_utf8_unchecked(&buf[..written]) }; + serializer.serialize_str(buf) } else { self.octets().serialize(serializer) } From 6814f978d72a6ae51f23e74baa2caf4e5b576f74 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 27 Nov 2022 16:56:31 -0800 Subject: [PATCH 3/3] Revert Buf::as_str safety change from PR 2319 --- serde/src/de/format.rs | 6 +++--- serde/src/de/mod.rs | 12 ++---------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/serde/src/de/format.rs b/serde/src/de/format.rs index aa4d53f98..f14580b8d 100644 --- a/serde/src/de/format.rs +++ b/serde/src/de/format.rs @@ -7,13 +7,13 @@ pub(super) struct Buf<'a> { } impl<'a> Buf<'a> { - pub(super) fn new(bytes: &'a mut [u8]) -> Self { + pub fn new(bytes: &'a mut [u8]) -> Self { Buf { bytes, offset: 0 } } - pub(super) unsafe fn as_str(&self) -> &str { + pub fn as_str(&self) -> &str { let slice = &self.bytes[..self.offset]; - str::from_utf8_unchecked(slice) + unsafe { str::from_utf8_unchecked(slice) } } } diff --git a/serde/src/de/mod.rs b/serde/src/de/mod.rs index 43084a486..d9dafbe1e 100644 --- a/serde/src/de/mod.rs +++ b/serde/src/de/mod.rs @@ -1376,11 +1376,7 @@ pub trait Visitor<'de>: Sized { let mut buf = [0u8; 58]; let mut writer = format::Buf::new(&mut buf); fmt::Write::write_fmt(&mut writer, format_args!("integer `{}` as i128", v)).unwrap(); - - // Safety: This is safe because we only wrote UTF-8 into the buffer. - let s = unsafe { writer.as_str() }; - - Err(Error::invalid_type(Unexpected::Other(s), &self)) + Err(Error::invalid_type(Unexpected::Other(writer.as_str()), &self)) } } @@ -1442,11 +1438,7 @@ pub trait Visitor<'de>: Sized { let mut buf = [0u8; 57]; let mut writer = format::Buf::new(&mut buf); fmt::Write::write_fmt(&mut writer, format_args!("integer `{}` as u128", v)).unwrap(); - - // Safety: This is safe because we only wrote UTF-8 into the buffer. - let s = unsafe { writer.as_str() }; - - Err(Error::invalid_type(Unexpected::Other(s), &self)) + Err(Error::invalid_type(Unexpected::Other(writer.as_str()), &self)) } }