Repository: ron-rs/ron
Pull request #109 (2018-05-31 15:08:11)

Use base64 for `serde_bytes` annotated fields
With the following `struct`, `small` will be represented as an array of `u8` and `large` as a base64 `String`.

``` rust
extern crate serde_derive;

extern crate serde;
extern crate serde_bytes;

#[derive(Serialize, Deserialize)]
struct BytesStruct {
    small: Vec<u8>,
    #[serde(with = "serde_bytes")]
    large: Vec<u8>,
}
```

fengalin: I'm glad you find this PR interesting! Is there anything I could do to help push this forward?
kvark: @fengalin sorry about the wait. Please rebase in the meantime :)
@torkleyy must have just missed this. PING!
kvark: oh I see the response now, oops, sorry @torkleyy  :)
@fengalin please bump the minor version and rebase
fengalin: @kvark no problem, I was just checking.
Rebased and bumped version.
fengalin: @torjkleyy I'm not quite sure about this. I think `base64` handling should only be selected when user explicitly uses `Bytes/_buf`.  I'll take a deeper look at this tomorrow.
torkleyy: The problem is that even though there's a `Value::Bytes` variant, it will
never be the result of a deserialization.

On Fri, Jun 8, 2018, 11:00 François Laignel <notifications@github.com>
wrote:

> @torjkleyy I'm not quite sure about this. I think base64 handling should
> only be selected when user explicitly uses Bytes/_buf. I'll take a deeper
> look at this tomorrow.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/ron-rs/ron/pull/109#issuecomment-395697688>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AWFFuT1J_fo7nj5hiLDaWhuYMUFPwG0pks5t6j0QgaJpZM4UVOFQ>
> .
>

fengalin: I marked the `Value::Bytes` variant as `unreachable!` and implemented `deserialize_bytes` and `deserialize_byte_buf`. Does it address your concerns @torkleyy?
fengalin: AFAIU, you will get the same value if the origin and target `struct` are annotated with `serde_bytes` or use `ByteBuf`. Are you saying we should use a specific notation in the serialized field to show this is base64?
torkleyy: Yes, with fields this holds, but not with `ron::Value`.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index e9c90147..61a75b67 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "ron"
-version = "0.2.2"
+version = "0.3.0"
 license = "MIT/Apache-2.0"
 keywords = ["parser", "serde", "serialization"]
 authors = [
@@ -18,8 +18,10 @@ exclude = ["bors.toml", ".travis.yml"]
 name = "ron"
 
 [dependencies]
+base64 = "^0.9.2"
 bitflags = "1"
 serde = { version = "1", features = ["serde_derive"] }
 
 [dev-dependencies]
+serde_bytes = "0.10"
 serde_json = "1"
diff --git a/src/de/error.rs b/src/de/error.rs
index 4de8651b..a8452862 100644
--- a/src/de/error.rs
+++ b/src/de/error.rs
@@ -4,6 +4,8 @@ use std::io;
 use std::str::Utf8Error;
 use std::string::FromUtf8Error;
 
+use base64;
+
 use serde::de;
 
 use parse::Position;
@@ -20,6 +22,7 @@ pub enum Error {
 
 #[derive(Clone, Debug, PartialEq)]
 pub enum ParseError {
+    Base64Error(base64::DecodeError),
     Eof,
     ExpectedArray,
     ExpectedArrayEnd,
@@ -79,6 +82,7 @@ impl StdError for Error {
             Error::IoError(ref s) => s,
             Error::Message(ref e) => e,
             Error::Parser(ref kind, _) => match *kind {
+                ParseError::Base64Error(ref e) => e.description(),
                 ParseError::Eof => "Unexpected end of file",
                 ParseError::ExpectedArray => "Expected array",
                 ParseError::ExpectedArrayEnd => "Expected end of array",
diff --git a/src/de/mod.rs b/src/de/mod.rs
index efe45eb9..9d168019 100644
--- a/src/de/mod.rs
+++ b/src/de/mod.rs
@@ -7,6 +7,8 @@ use std::borrow::Cow;
 use std::io;
 use std::str;
 
+use base64;
+
 use serde::de::{self, DeserializeSeed, Deserializer as Deserializer_, Visitor};
 
 use self::id::IdDeserializer;
@@ -235,14 +237,28 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
     where
         V: Visitor<'de>,
     {
-        self.deserialize_seq(visitor)
+        self.deserialize_byte_buf(visitor)
     }
 
     fn deserialize_byte_buf<V>(self, visitor: V) -> Result<V::Value>
     where
         V: Visitor<'de>,
     {
-        self.deserialize_seq(visitor)
+        use parse::ParsedStr;
+
+        let res = {
+            let string = self.bytes.string()?;
+            let base64_str = match string {
+                ParsedStr::Allocated(ref s) => s.as_str(),
+                ParsedStr::Slice(ref s) => s,
+            };
+            base64::decode(base64_str)
+        };
+
+        match res {
+            Ok(byte_buf) => visitor.visit_byte_buf(byte_buf),
+            Err(err) => self.bytes.err(ParseError::Base64Error(err))
+        }
     }
 
     fn deserialize_option<V>(self, visitor: V) -> Result<V::Value>
@@ -443,7 +459,10 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
     where
         V: Visitor<'de>,
     {
-        visitor.visit_bytes(self.bytes.identifier()?)
+        visitor.visit_str(
+            str::from_utf8(self.bytes.identifier()?)
+                .map_err(|e| self.bytes.error(e.into()))?
+        )
     }
 
     fn deserialize_ignored_any<V>(self, visitor: V) -> Result<V::Value>
diff --git a/src/lib.rs b/src/lib.rs
index cf260279..33257c07 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -59,6 +59,8 @@ Serializing / Deserializing is as simple as calling `to_string` / `from_str`.
 
 #![deny(unused_must_use)]
 
+extern crate base64;
+
 #[macro_use]
 extern crate bitflags;
 #[macro_use]
diff --git a/src/ser/mod.rs b/src/ser/mod.rs
index 3b1a8b63..97f11945 100644
--- a/src/ser/mod.rs
+++ b/src/ser/mod.rs
@@ -2,6 +2,8 @@ use std::error::Error as StdError;
 use std::fmt::{Display, Formatter, Write, Result as FmtResult};
 use std::result::Result as StdResult;
 
+use base64;
+
 use serde::ser::{self, Serialize};
 
 mod value;
@@ -260,13 +262,7 @@ impl<'a> ser::Serializer for &'a mut Serializer {
     }
 
     fn serialize_bytes(self, v: &[u8]) -> Result<()> {
-        use serde::ser::SerializeSeq;
-        //TODO: shorter version? e.g. base64 encoding in a single line
-        let mut seq = self.serialize_seq(Some(v.len()))?;
-        for byte in v {
-            seq.serialize_element(byte)?;
-        }
-        seq.end()
+        self.serialize_str(base64::encode(v).as_str())
     }
 
     fn serialize_none(self) -> Result<()> {
@@ -741,4 +737,22 @@ mod tests {
     fn test_escape() {
         assert_eq!(to_string(&r#""Quoted""#).unwrap(), r#""\"Quoted\"""#);
     }
+
+    #[test]
+    fn test_byte_stream() {
+        extern crate serde_bytes;
+
+        let small: [u8; 16] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
+        assert_eq!(to_string(&small).unwrap(), "(0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,)");
+
+        let large = vec![255u8; 64];
+        let large = serde_bytes::Bytes::new(&large);
+        assert_eq!(
+            to_string(&large).unwrap(),
+            concat!(
+                "\"/////////////////////////////////////////",
+                "////////////////////////////////////////////w==\""
+            )
+        );
+    }
 }


Test patch:
diff --git a/src/de/tests.rs b/src/de/tests.rs
index 3ab1334d..152c2410 100644
--- a/src/de/tests.rs
+++ b/src/de/tests.rs
@@ -1,3 +1,5 @@
+extern crate serde_bytes;
+
 use super::*;
 
 #[derive(Debug, PartialEq, Deserialize)]
@@ -20,6 +22,14 @@ enum MyEnum {
     D { a: i32, b: i32 },
 }
 
+
+#[derive(Debug, Deserialize, PartialEq)]
+struct BytesStruct {
+    small: Vec<u8>,
+    #[serde(with = "serde_bytes")]
+    large: Vec<u8>,
+}
+
 #[test]
 fn test_empty_struct() {
     assert_eq!(Ok(EmptyStruct1), from_str("EmptyStruct1"));
@@ -274,3 +284,11 @@ fn implicit_some() {
 fn ws_tuple_newtype_variant() {
     assert_eq!(Ok(MyEnum::B(true)), from_str("B  ( \n true \n ) "));
 }
+
+#[test]
+fn test_byte_stream() {
+    assert_eq!(
+        Ok(BytesStruct{ small: vec![1, 2], large: vec![1, 2, 3, 4] }),
+        from_str("BytesStruct( small:[1, 2], large:\"AQIDBA==\" )"),
+    );
+}