From 857974ab8a1e04f865ad8d02d7a52aa56650583b Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 16:09:37 -0500 Subject: [PATCH 1/8] impls for null-terminated FFI string types Fixes #800. --- serde/src/de/impls.rs | 29 +++++++++++++++++++++++++++++ serde/src/ser/impls.rs | 24 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index ba0acae2..1ad97538 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -25,6 +25,8 @@ use std::net; #[cfg(feature = "std")] use std::path; use core::str; +#[cfg(feature = "std")] +use std::ffi::{CStr, CString}; #[cfg(feature = "std")] use std::rc::Rc; @@ -295,6 +297,33 @@ impl Deserialize for String { /////////////////////////////////////////////////////////////////////////////// +#[cfg(feature = "std")] +impl Deserialize for Box { + fn deserialize(deserializer: D) -> Result + where D: Deserializer + { + use std::mem; + let s = try!(CString::deserialize(deserializer)); + let slice = s.into_bytes_with_nul().into_boxed_slice(); + Ok(unsafe { mem::transmute::, Box>(slice) }) + } +} + + +#[cfg(feature = "std")] +impl Deserialize for CString { + fn deserialize(deserializer: D) -> Result + where D: Deserializer + { + let mut v: Vec = try!(Deserialize::deserialize(deserializer)); + v.pop(); // cut trailing NULL, because CString::new adds it + CString::new(v) + .map_err(|e| Error::custom(format!("unexpected NULL at byte {}", e.nul_position()))) + } +} + +/////////////////////////////////////////////////////////////////////////////// + struct OptionVisitor { marker: PhantomData, } diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index a7ac780f..32eb716f 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -22,6 +22,8 @@ use core::ops; #[cfg(feature = "std")] use std::path; #[cfg(feature = "std")] +use std::ffi::{CString, CStr}; +#[cfg(feature = "std")] use std::rc::Rc; #[cfg(all(feature = "alloc", not(feature = "std")))] use alloc::rc::Rc; @@ -98,6 +100,28 @@ impl Serialize for String { /////////////////////////////////////////////////////////////////////////////// +#[cfg(feature = "std")] +impl Serialize for CStr { + #[inline] + fn serialize(&self, serializer: S) -> Result + where S: Serializer + { + (self.to_bytes_with_nul()).serialize(serializer) + } +} + +#[cfg(feature = "std")] +impl Serialize for CString { + #[inline] + fn serialize(&self, serializer: S) -> Result + where S: Serializer + { + (self.to_bytes_with_nul()).serialize(serializer) + } +} + +/////////////////////////////////////////////////////////////////////////////// + impl Serialize for Option where T: Serialize { From 9f83164c4071753236909326c1d7760affbb6e6e Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 17:14:28 -0500 Subject: [PATCH 2/8] Don't serialize trailing NULL --- serde/src/de/impls.rs | 3 +-- serde/src/ser/impls.rs | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index 1ad97538..6a8c1a77 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -315,8 +315,7 @@ impl Deserialize for CString { fn deserialize(deserializer: D) -> Result where D: Deserializer { - let mut v: Vec = try!(Deserialize::deserialize(deserializer)); - v.pop(); // cut trailing NULL, because CString::new adds it + let v: Vec = try!(Deserialize::deserialize(deserializer)); CString::new(v) .map_err(|e| Error::custom(format!("unexpected NULL at byte {}", e.nul_position()))) } diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index 32eb716f..7d44ab34 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -106,7 +106,7 @@ impl Serialize for CStr { fn serialize(&self, serializer: S) -> Result where S: Serializer { - (self.to_bytes_with_nul()).serialize(serializer) + self.to_bytes().serialize(serializer) } } @@ -116,7 +116,7 @@ impl Serialize for CString { fn serialize(&self, serializer: S) -> Result where S: Serializer { - (self.to_bytes_with_nul()).serialize(serializer) + self.to_bytes().serialize(serializer) } } From fc9d78e26b82b1ad7bb8b66f4a4f9a75c420c44a Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 17:14:39 -0500 Subject: [PATCH 3/8] Use serialize_bytes for speed --- serde/src/ser/impls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index 7d44ab34..4c7a263f 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -106,7 +106,7 @@ impl Serialize for CStr { fn serialize(&self, serializer: S) -> Result where S: Serializer { - self.to_bytes().serialize(serializer) + serializer.serialize_bytes(self.to_bytes()) } } @@ -116,7 +116,7 @@ impl Serialize for CString { fn serialize(&self, serializer: S) -> Result where S: Serializer { - self.to_bytes().serialize(serializer) + serializer.serialize_bytes(self.to_bytes()) } } From be09fc9bbb6fae9272c81b160e97711be5fd1243 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 17:33:56 -0500 Subject: [PATCH 4/8] Remove unsafe Deserialize impl for CStr See also https://github.com/rust-lang/rust/issues/40248 --- serde/src/de/impls.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index 6a8c1a77..390a94e1 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -297,19 +297,6 @@ impl Deserialize for String { /////////////////////////////////////////////////////////////////////////////// -#[cfg(feature = "std")] -impl Deserialize for Box { - fn deserialize(deserializer: D) -> Result - where D: Deserializer - { - use std::mem; - let s = try!(CString::deserialize(deserializer)); - let slice = s.into_bytes_with_nul().into_boxed_slice(); - Ok(unsafe { mem::transmute::, Box>(slice) }) - } -} - - #[cfg(feature = "std")] impl Deserialize for CString { fn deserialize(deserializer: D) -> Result From 0d6d077e6a01cd2db75af9eab78f1fa55ebfe5cc Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 18:05:08 -0500 Subject: [PATCH 5/8] Serialize and deserialize CString through [u8] --- serde/src/de/impls.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index 390a94e1..892b542e 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -26,7 +26,7 @@ use std::net; use std::path; use core::str; #[cfg(feature = "std")] -use std::ffi::{CStr, CString}; +use std::ffi::CString; #[cfg(feature = "std")] use std::rc::Rc; @@ -55,6 +55,8 @@ use de::{Deserialize, Deserializer, EnumVisitor, Error, MapVisitor, SeqVisitor, VariantVisitor, Visitor}; use de::from_primitive::FromPrimitive; +use super::super::bytes::ByteBuf; + /////////////////////////////////////////////////////////////////////////////// /// A visitor that produces a `()`. @@ -302,7 +304,7 @@ impl Deserialize for CString { fn deserialize(deserializer: D) -> Result where D: Deserializer { - let v: Vec = try!(Deserialize::deserialize(deserializer)); + let v: Vec = try!(ByteBuf::deserialize(deserializer)).into(); CString::new(v) .map_err(|e| Error::custom(format!("unexpected NULL at byte {}", e.nul_position()))) } From d90eecd4a2c4b320e3ae2d06a56e99c2654f4997 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 18:06:04 -0500 Subject: [PATCH 6/8] Add tests for CStr(ing) ser/de --- test_suite/tests/test_de.rs | 18 ++++++++++++++++++ test_suite/tests/test_ser.rs | 11 +++++++++++ 2 files changed, 29 insertions(+) diff --git a/test_suite/tests/test_de.rs b/test_suite/tests/test_de.rs index f4112428..8e79f1f1 100644 --- a/test_suite/tests/test_de.rs +++ b/test_suite/tests/test_de.rs @@ -6,6 +6,7 @@ use std::net; use std::path::PathBuf; use std::time::Duration; use std::default::Default; +use std::ffi::CString; extern crate serde; use serde::Deserialize; @@ -878,6 +879,11 @@ declare_tests! { Token::String("/usr/local/lib".to_owned()), ], } + test_cstring { + CString::new("abc").unwrap() => &[ + Token::Bytes(b"abc"), + ], + } } #[cfg(feature = "unstable")] @@ -995,4 +1001,16 @@ declare_error_tests! { ], Error::Message("invalid length 1, expected an array of length 3".into()), } + test_cstring_internal_null { + &[ + Token::Bytes(b"a\0c"), + ], + Error::Message("unexpected NULL at byte 1".into()), + } + test_cstring_internal_null_end { + &[ + Token::Bytes(b"ac\0"), + ], + Error::Message("unexpected NULL at byte 2".into()), + } } diff --git a/test_suite/tests/test_ser.rs b/test_suite/tests/test_ser.rs index e1f298ea..72ed3f0c 100644 --- a/test_suite/tests/test_ser.rs +++ b/test_suite/tests/test_ser.rs @@ -6,6 +6,7 @@ use std::net; use std::path::{Path, PathBuf}; use std::str; use std::time::Duration; +use std::ffi::CString; extern crate serde; @@ -389,6 +390,16 @@ declare_tests! { Token::Str("/usr/local/lib"), ], } + test_cstring { + CString::new("abc").unwrap() => &[ + Token::Bytes(b"abc"), + ], + } + test_cstr { + (&*CString::new("abc").unwrap()) => &[ + Token::Bytes(b"abc"), + ], + } } From defcbef7ab133c7bca996ef01430f0acd99e64cb Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 23:28:35 -0500 Subject: [PATCH 7/8] Use a non-stupid path for bytes::ByteBuf --- serde/src/de/impls.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index 892b542e..340bd0f7 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -55,7 +55,7 @@ use de::{Deserialize, Deserializer, EnumVisitor, Error, MapVisitor, SeqVisitor, VariantVisitor, Visitor}; use de::from_primitive::FromPrimitive; -use super::super::bytes::ByteBuf; +use bytes::ByteBuf; /////////////////////////////////////////////////////////////////////////////// From d294a10e837fc0dab7818b95111db0d1214135a5 Mon Sep 17 00:00:00 2001 From: Jon Gjengset Date: Fri, 3 Mar 2017 23:48:00 -0500 Subject: [PATCH 8/8] Only include ByteBuf when ser/de is on for std --- serde/src/de/impls.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/serde/src/de/impls.rs b/serde/src/de/impls.rs index 340bd0f7..2946dcfe 100644 --- a/serde/src/de/impls.rs +++ b/serde/src/de/impls.rs @@ -55,6 +55,7 @@ use de::{Deserialize, Deserializer, EnumVisitor, Error, MapVisitor, SeqVisitor, VariantVisitor, Visitor}; use de::from_primitive::FromPrimitive; +#[cfg(feature = "std")] use bytes::ByteBuf; ///////////////////////////////////////////////////////////////////////////////