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

@@ -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<S>(&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<S>(&self, serializer: &mut S) -> Result<(), S::Error>
         where S: Serializer,
     {
-        self.to_str().unwrap().serialize(serializer)
+        self.as_path().serialize(serializer)
     }
 }
 

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>;

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 }
 

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 <20>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 <20>World")],
+        token::Error::InvalidValue("Path contains invalid UTF-8 characters".to_owned()));
+}

serde_tests/tests/token.rs

@@ -64,7 +64,7 @@ impl<'a, I> Serializer<I>
         }
     }
 
-    fn visit_sequence<V>(&mut self, mut visitor: V) -> Result<(), ()>
+    fn visit_sequence<V>(&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<I>
         Ok(())
     }
 
-    fn visit_mapping<V>(&mut self, mut visitor: V) -> Result<(), ()>
+    fn visit_mapping<V>(&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<I>
 impl<'a, I> ser::Serializer for Serializer<I>
     where I: Iterator<Item=&'a Token<'a>>,
 {
-    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<I>
                                 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<I>
     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<V>(&mut self, value: V) -> Result<(), ()>
+    fn serialize_some<V>(&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<I>
     }
 
 
-    fn serialize_seq<V>(&mut self, visitor: V) -> Result<(), ()>
+    fn serialize_seq<V>(&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<I>
 
     fn serialize_newtype_struct<T>(&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<V>(&mut self, name: &str, visitor: V) -> Result<(), ()>
+    fn serialize_tuple_struct<V>(&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<I>
                               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<I>
         self.visit_sequence(visitor)
     }
 
-    fn serialize_seq_elt<T>(&mut self, value: T) -> Result<(), ()>
+    fn serialize_seq_elt<T>(&mut self, value: T) -> Result<(), Error>
         where T: ser::Serialize
     {
         assert_eq!(self.tokens.next(), Some(&Token::SeqSep));
         value.serialize(self)
     }
 
-    fn serialize_map<V>(&mut self, visitor: V) -> Result<(), ()>
+    fn serialize_map<V>(&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<I>
         self.visit_mapping(visitor)
     }
 
-    fn serialize_struct<V>(&mut self, name: &str, visitor: V) -> Result<(), ()>
+    fn serialize_struct<V>(&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<I>
                                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<I>
         self.visit_mapping(visitor)
     }
 
-    fn serialize_map_elt<K, V>(&mut self, key: K, value: V) -> Result<(), ()>
+    fn serialize_map_elt<K, V>(&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<T>(value: &T, tokens: &[Token])
     assert_eq!(ser.tokens.next(), None);
 }
 
+// Expect an error deserializing tokens into a T
+pub fn assert_ser_tokens_error<T>(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<T>(value: &T, tokens: Vec<Token<'static>>)
     where T: de::Deserialize + PartialEq + fmt::Debug,
 {
@@ -804,7 +822,7 @@ pub fn assert_de_tokens_error<T>(tokens: Vec<Token<'static>>, error: Error)
 {
     let mut de = Deserializer::new(tokens.into_iter());
     let v: Result<T, Error> = 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