From f1b20577d33aeb7ea53398cccda97800408107d1 Mon Sep 17 00:00:00 2001 From: Erick Tryzelaar Date: Thu, 28 Jan 2016 10:41:21 -0800 Subject: [PATCH] feat(ser): Add ser::Error trait; avoid panic when serializing Paths The only way to safely serialize a `Path` is to use `.to_string_lossy()`, which replaces invalid UTF-8 characters with the U+FFFD replacement character. Unfortunately this would lose information, so for our default implementations, it'd be better to punt and report an error, and leave it up to the user to decide if they want to use the lossy encoding. Unfortunately, we had no way for `Serializer`s to require some methods on `Serializer::Error`, so there was no way before this patch for the `Path` implementation to generically report that it cannot encode this value. This adds that implementation. breaking-change Closes #57. --- serde/src/ser/impls.rs | 8 ++- serde/src/ser/mod.rs | 18 ++++++- serde_tests/tests/test_bytes.rs | 6 +++ serde_tests/tests/test_ser.rs | 22 ++++++++- serde_tests/tests/token.rs | 86 ++++++++++++++++++++------------- 5 files changed, 102 insertions(+), 38 deletions(-) diff --git a/serde/src/ser/impls.rs b/serde/src/ser/impls.rs index 7d01a384..cc20f9b6 100644 --- a/serde/src/ser/impls.rs +++ b/serde/src/ser/impls.rs @@ -27,6 +27,7 @@ use std::sync::Arc; use core::nonzero::{NonZero, Zeroable}; use super::{ + Error, Serialize, Serializer, SeqVisitor, @@ -682,7 +683,10 @@ impl Serialize for path::Path { fn serialize(&self, serializer: &mut S) -> Result<(), S::Error> where S: Serializer, { - self.to_str().unwrap().serialize(serializer) + match self.to_str() { + Some(s) => s.serialize(serializer), + None => Err(Error::invalid_value("Path contains invalid UTF-8 characters")), + } } } @@ -690,7 +694,7 @@ impl Serialize for path::PathBuf { fn serialize(&self, serializer: &mut S) -> Result<(), S::Error> where S: Serializer, { - self.to_str().unwrap().serialize(serializer) + self.as_path().serialize(serializer) } } diff --git a/serde/src/ser/mod.rs b/serde/src/ser/mod.rs index 19dd8212..64bdbdfa 100644 --- a/serde/src/ser/mod.rs +++ b/serde/src/ser/mod.rs @@ -1,9 +1,25 @@ //! Generic serialization framework. +use std::error; + pub mod impls; /////////////////////////////////////////////////////////////////////////////// +/// `Error` is a trait that allows a `Serialize` to generically create a +/// `Serializer` error. +pub trait Error: Sized + error::Error { + /// Raised when there is general error when deserializing a type. + fn syntax(msg: &str) -> Self; + + /// Raised when a `Serialize` was passed an incorrect value. + fn invalid_value(msg: &str) -> Self { + Error::syntax(msg) + } +} + +/////////////////////////////////////////////////////////////////////////////// + /// A trait that describes a type that can be serialized by a `Serializer`. pub trait Serialize { /// Serializes this value into this serializer. @@ -16,7 +32,7 @@ pub trait Serialize { /// A trait that describes a type that can serialize a stream of values into the underlying format. pub trait Serializer { /// The error type that can be returned if some error occurs during serialization. - type Error; + type Error: Error; /// Serializes a `bool` value. fn serialize_bool(&mut self, v: bool) -> Result<(), Self::Error>; diff --git a/serde_tests/tests/test_bytes.rs b/serde_tests/tests/test_bytes.rs index 137d0098..785ff550 100644 --- a/serde_tests/tests/test_bytes.rs +++ b/serde_tests/tests/test_bytes.rs @@ -9,6 +9,12 @@ use serde::bytes::{ByteBuf, Bytes}; #[derive(Debug, PartialEq)] struct Error; +impl serde::ser::Error for Error { + fn syntax(_: &str) -> Error { Error } + + fn invalid_value(_field: &str) -> Error { Error } +} + impl serde::de::Error for Error { fn syntax(_: &str) -> Error { Error } diff --git a/serde_tests/tests/test_ser.rs b/serde_tests/tests/test_ser.rs index 7fbae4e4..77bf3eb8 100644 --- a/serde_tests/tests/test_ser.rs +++ b/serde_tests/tests/test_ser.rs @@ -1,12 +1,13 @@ use std::collections::BTreeMap; use std::path::{Path, PathBuf}; +use std::str; use num::FromPrimitive; use num::bigint::{BigInt, BigUint}; use num::complex::Complex; use num::rational::Ratio; -use token::Token; +use token::{self, Token}; ////////////////////////////////////////////////////////////////////////// @@ -305,3 +306,22 @@ declare_ser_tests! { ], } } + +#[test] +fn test_cannot_serialize_paths() { + let path = unsafe { + str::from_utf8_unchecked(b"Hello \xF0\x90\x80World") + }; + token::assert_ser_tokens_error( + &Path::new(path), + &[Token::Str("Hello �World")], + token::Error::InvalidValue("Path contains invalid UTF-8 characters".to_owned())); + + let mut path_buf = PathBuf::new(); + path_buf.push(path); + + token::assert_ser_tokens_error( + &path_buf, + &[Token::Str("Hello �World")], + token::Error::InvalidValue("Path contains invalid UTF-8 characters".to_owned())); +} diff --git a/serde_tests/tests/token.rs b/serde_tests/tests/token.rs index 43502567..b1170511 100644 --- a/serde_tests/tests/token.rs +++ b/serde_tests/tests/token.rs @@ -64,7 +64,7 @@ impl<'a, I> Serializer } } - fn visit_sequence(&mut self, mut visitor: V) -> Result<(), ()> + fn visit_sequence(&mut self, mut visitor: V) -> Result<(), Error> where V: ser::SeqVisitor { while let Some(()) = try!(visitor.visit(self)) { } @@ -74,7 +74,7 @@ impl<'a, I> Serializer Ok(()) } - fn visit_mapping(&mut self, mut visitor: V) -> Result<(), ()> + fn visit_mapping(&mut self, mut visitor: V) -> Result<(), Error> where V: ser::MapVisitor { while let Some(()) = try!(visitor.visit(self)) { } @@ -88,9 +88,9 @@ impl<'a, I> Serializer impl<'a, I> ser::Serializer for Serializer where I: Iterator>, { - type Error = (); + type Error = Error; - fn serialize_unit(&mut self) -> Result<(), ()> { + fn serialize_unit(&mut self) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::Unit)); Ok(()) } @@ -99,14 +99,14 @@ impl<'a, I> ser::Serializer for Serializer name: &str, _variant_index: usize, variant: &str, - value: T) -> Result<(), ()> + value: T) -> Result<(), Error> where T: ser::Serialize, { assert_eq!(self.tokens.next(), Some(&Token::EnumNewtype(name, variant))); value.serialize(self) } - fn serialize_unit_struct(&mut self, name: &str) -> Result<(), ()> { + fn serialize_unit_struct(&mut self, name: &str) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::UnitStruct(name))); Ok(()) } @@ -114,93 +114,93 @@ impl<'a, I> ser::Serializer for Serializer fn serialize_unit_variant(&mut self, name: &str, _variant_index: usize, - variant: &str) -> Result<(), ()> { + variant: &str) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::EnumUnit(name, variant))); Ok(()) } - fn serialize_bool(&mut self, v: bool) -> Result<(), ()> { + fn serialize_bool(&mut self, v: bool) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::Bool(v))); Ok(()) } - fn serialize_isize(&mut self, v: isize) -> Result<(), ()> { + fn serialize_isize(&mut self, v: isize) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::Isize(v))); Ok(()) } - fn serialize_i8(&mut self, v: i8) -> Result<(), ()> { + fn serialize_i8(&mut self, v: i8) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::I8(v))); Ok(()) } - fn serialize_i16(&mut self, v: i16) -> Result<(), ()> { + fn serialize_i16(&mut self, v: i16) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::I16(v))); Ok(()) } - fn serialize_i32(&mut self, v: i32) -> Result<(), ()> { + fn serialize_i32(&mut self, v: i32) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::I32(v))); Ok(()) } - fn serialize_i64(&mut self, v: i64) -> Result<(), ()> { + fn serialize_i64(&mut self, v: i64) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::I64(v))); Ok(()) } - fn serialize_usize(&mut self, v: usize) -> Result<(), ()> { + fn serialize_usize(&mut self, v: usize) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::Usize(v))); Ok(()) } - fn serialize_u8(&mut self, v: u8) -> Result<(), ()> { + fn serialize_u8(&mut self, v: u8) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::U8(v))); Ok(()) } - fn serialize_u16(&mut self, v: u16) -> Result<(), ()> { + fn serialize_u16(&mut self, v: u16) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::U16(v))); Ok(()) } - fn serialize_u32(&mut self, v: u32) -> Result<(), ()> { + fn serialize_u32(&mut self, v: u32) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::U32(v))); Ok(()) } - fn serialize_u64(&mut self, v: u64) -> Result<(), ()> { + fn serialize_u64(&mut self, v: u64) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::U64(v))); Ok(()) } - fn serialize_f32(&mut self, v: f32) -> Result<(), ()> { + fn serialize_f32(&mut self, v: f32) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::F32(v))); Ok(()) } - fn serialize_f64(&mut self, v: f64) -> Result<(), ()> { + fn serialize_f64(&mut self, v: f64) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::F64(v))); Ok(()) } - fn serialize_char(&mut self, v: char) -> Result<(), ()> { + fn serialize_char(&mut self, v: char) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::Char(v))); Ok(()) } - fn serialize_str(&mut self, v: &str) -> Result<(), ()> { + fn serialize_str(&mut self, v: &str) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::Str(v))); Ok(()) } - fn serialize_none(&mut self) -> Result<(), ()> { + fn serialize_none(&mut self) -> Result<(), Error> { assert_eq!(self.tokens.next(), Some(&Token::Option(false))); Ok(()) } - fn serialize_some(&mut self, value: V) -> Result<(), ()> + fn serialize_some(&mut self, value: V) -> Result<(), Error> where V: ser::Serialize, { assert_eq!(self.tokens.next(), Some(&Token::Option(true))); @@ -208,7 +208,7 @@ impl<'a, I> ser::Serializer for Serializer } - fn serialize_seq(&mut self, visitor: V) -> Result<(), ()> + fn serialize_seq(&mut self, visitor: V) -> Result<(), Error> where V: ser::SeqVisitor { let len = visitor.len(); @@ -220,14 +220,14 @@ impl<'a, I> ser::Serializer for Serializer fn serialize_newtype_struct(&mut self, name: &'static str, - value: T) -> Result<(), ()> + value: T) -> Result<(), Error> where T: ser::Serialize, { assert_eq!(self.tokens.next(), Some(&Token::StructNewtype(name))); value.serialize(self) } - fn serialize_tuple_struct(&mut self, name: &str, visitor: V) -> Result<(), ()> + fn serialize_tuple_struct(&mut self, name: &str, visitor: V) -> Result<(), Error> where V: ser::SeqVisitor { let len = visitor.len(); @@ -241,7 +241,7 @@ impl<'a, I> ser::Serializer for Serializer name: &str, _variant_index: usize, variant: &str, - visitor: V) -> Result<(), ()> + visitor: V) -> Result<(), Error> where V: ser::SeqVisitor { let len = visitor.len(); @@ -251,14 +251,14 @@ impl<'a, I> ser::Serializer for Serializer self.visit_sequence(visitor) } - fn serialize_seq_elt(&mut self, value: T) -> Result<(), ()> + fn serialize_seq_elt(&mut self, value: T) -> Result<(), Error> where T: ser::Serialize { assert_eq!(self.tokens.next(), Some(&Token::SeqSep)); value.serialize(self) } - fn serialize_map(&mut self, visitor: V) -> Result<(), ()> + fn serialize_map(&mut self, visitor: V) -> Result<(), Error> where V: ser::MapVisitor { let len = visitor.len(); @@ -268,7 +268,7 @@ impl<'a, I> ser::Serializer for Serializer self.visit_mapping(visitor) } - fn serialize_struct(&mut self, name: &str, visitor: V) -> Result<(), ()> + fn serialize_struct(&mut self, name: &str, visitor: V) -> Result<(), Error> where V: ser::MapVisitor { let len = visitor.len(); @@ -282,7 +282,7 @@ impl<'a, I> ser::Serializer for Serializer name: &str, _variant_index: usize, variant: &str, - visitor: V) -> Result<(), ()> + visitor: V) -> Result<(), Error> where V: ser::MapVisitor { let len = visitor.len(); @@ -292,7 +292,7 @@ impl<'a, I> ser::Serializer for Serializer self.visit_mapping(visitor) } - fn serialize_map_elt(&mut self, key: K, value: V) -> Result<(), ()> + fn serialize_map_elt(&mut self, key: K, value: V) -> Result<(), Error> where K: ser::Serialize, V: ser::Serialize, { @@ -316,10 +316,19 @@ pub enum Error { UnknownFieldError(String), MissingFieldError(&'static str), InvalidName(&'static str), + InvalidValue(String), UnexpectedToken(Token<'static>), ValueError(value::Error), } +impl ser::Error for Error { + fn syntax(_: &str) -> Error { Error::SyntaxError } + + fn invalid_value(msg: &str) -> Error { + Error::InvalidValue(msg.to_owned()) + } +} + impl de::Error for Error { fn syntax(_: &str) -> Error { Error::SyntaxError } @@ -789,6 +798,15 @@ pub fn assert_ser_tokens(value: &T, tokens: &[Token]) assert_eq!(ser.tokens.next(), None); } +// Expect an error deserializing tokens into a T +pub fn assert_ser_tokens_error(value: &T, tokens: &[Token], error: Error) + where T: ser::Serialize + PartialEq + fmt::Debug, +{ + let mut ser = Serializer::new(tokens.iter()); + let v: Result<(), Error> = ser::Serialize::serialize(value, &mut ser); + assert_eq!(v.as_ref(), Err(&error)); +} + pub fn assert_de_tokens(value: &T, tokens: Vec>) where T: de::Deserialize + PartialEq + fmt::Debug, { @@ -804,7 +822,7 @@ pub fn assert_de_tokens_error(tokens: Vec>, error: Error) { let mut de = Deserializer::new(tokens.into_iter()); let v: Result = de::Deserialize::deserialize(&mut de); - assert_eq!(v.as_ref(), Err(&error)); + assert_eq!(v, Err(error)); } // Tests that the given token stream is ignorable when embedded in