Repository: pyfisch/cbor (rust)

Issue:
What if I don't want the packed serialization?
I'm trying to avoid having a format that inadvertently works. While avoiding serialization is a matter of just not calling the `_packed` functions, deserialization seems to implicitly always use it.

E.g., coding against a spec, I do not want other, incorrect (according to the spec, which might be agnostic to Rust/`serde_cbor`) to successfully deserialize, lest someone thinks that format *is* supported.

Pull request:
packed encoding deserialization can now be turned off
Fixes #112 
See #119 

Patch:
diff --git a/src/de.rs b/src/de.rs
index b2fd156..d7babaf 100644
--- a/src/de.rs
+++ b/src/de.rs
@@ -442,8 +442,15 @@ where
     where
         V: de::Visitor<'de>,
     {
+        let accept_packed = self.accept_packed;
+        let accept_named = self.accept_named;
         self.recursion_checked(|de| {
-            let value = visitor.visit_map(MapAccess { de, len: &mut len })?;
+            let value = visitor.visit_map(MapAccess {
+                de,
+                len: &mut len,
+                accept_named,
+                accept_packed,
+            })?;
 
             if len != 0 {
                 Err(de.error(ErrorCode::TrailingData))
@@ -457,8 +464,14 @@ where
     where
         V: de::Visitor<'de>,
     {
+        let accept_named = self.accept_named;
+        let accept_packed = self.accept_packed;
         self.recursion_checked(|de| {
-            let value = visitor.visit_map(IndefiniteMapAccess { de })?;
+            let value = visitor.visit_map(IndefiniteMapAccess {
+                de,
+                accept_packed,
+                accept_named,
+            })?;
             match de.next()? {
                 Some(0xff) => Ok(value),
                 Some(_) => Err(de.error(ErrorCode::TrailingData)),
@@ -488,10 +501,17 @@ where
     where
         V: de::Visitor<'de>,
     {
+        let accept_named = self.accept_named;
+        let accept_packed = self.accept_packed;
         self.recursion_checked(|de| {
             let mut len = 1;
             let value = visitor.visit_enum(VariantAccessMap {
-                map: MapAccess { de, len: &mut len },
+                map: MapAccess {
+                    de,
+                    len: &mut len,
+                    accept_packed,
+                    accept_named,
+                },
             })?;
 
             if len != 0 {
@@ -930,6 +950,8 @@ where
 struct MapAccess<'a, R> {
     de: &'a mut Deserializer<R>,
     len: &'a mut usize,
+    accept_named: bool,
+    accept_packed: bool,
 }
 
 impl<'de, 'a, R> de::MapAccess<'de> for MapAccess<'a, R>
@@ -947,6 +969,16 @@ where
         }
         *self.len -= 1;
 
+        match self.de.peek()? {
+            Some(_byte @ 0x00..=0x1b) if !self.accept_packed => {
+                return Err(self.de.error(ErrorCode::WrongStructFormat));
+            }
+            Some(_byte @ 0x60..=0x7f) if !self.accept_named => {
+                return Err(self.de.error(ErrorCode::WrongStructFormat));
+            }
+            _ => {}
+        };
+
         let value = seed.deserialize(&mut *self.de)?;
         Ok(Some(value))
     }
@@ -974,6 +1006,8 @@ where
 
 struct IndefiniteMapAccess<'a, R> {
     de: &'a mut Deserializer<R>,
+    accept_packed: bool,
+    accept_named: bool,
 }
 
 impl<'de, 'a, R> de::MapAccess<'de> for IndefiniteMapAccess<'a, R>
@@ -987,6 +1021,12 @@ where
         K: de::DeserializeSeed<'de>,
     {
         match self.de.peek()? {
+            Some(_byte @ 0x00..=0x1b) if !self.accept_packed => {
+                return Err(self.de.error(ErrorCode::WrongStructFormat))
+            }
+            Some(_byte @ 0x60..=0x7f) if !self.accept_named => {
+                return Err(self.de.error(ErrorCode::WrongStructFormat))
+            }
             Some(0xff) => return Ok(None),
             Some(_) => {}
             None => return Err(self.de.error(ErrorCode::EofWhileParsingMap)),
diff --git a/src/error.rs b/src/error.rs
index d696722..0667afa 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -144,7 +144,8 @@ impl Error {
             | ErrorCode::ArrayTooShort
             | ErrorCode::ArrayTooLong
             | ErrorCode::RecursionLimitExceeded
-            | ErrorCode::WrongEnumFormat => Category::Syntax,
+            | ErrorCode::WrongEnumFormat
+            | ErrorCode::WrongStructFormat => Category::Syntax,
         }
     }
 
@@ -283,6 +284,7 @@ pub(crate) enum ErrorCode {
     ArrayTooLong,
     RecursionLimitExceeded,
     WrongEnumFormat,
+    WrongStructFormat,
 }
 
 impl fmt::Display for ErrorCode {
@@ -309,6 +311,7 @@ impl fmt::Display for ErrorCode {
             ErrorCode::ArrayTooLong => f.write_str("array too long"),
             ErrorCode::RecursionLimitExceeded => f.write_str("recursion limit exceeded"),
             ErrorCode::WrongEnumFormat => f.write_str("wrong enum format"),
+            ErrorCode::WrongStructFormat => f.write_str("wrong struct format"),
         }
     }
 }


Test patch:
diff --git a/tests/de.rs b/tests/de.rs
index cea0343..8eed7cd 100644
--- a/tests/de.rs
+++ b/tests/de.rs
@@ -403,21 +403,79 @@ mod std_tests {
         assert_eq!(rest, &[]);
     }
 
-    fn from_slice_stream_options<'a, T>(
-        slice: &'a [u8],
+    #[derive(Debug, Copy, Clone)]
+    struct Options {
         standard: bool,
         legacy: bool,
+        packed: bool,
+        named: bool,
+    }
+
+    impl Default for Options {
+        fn default() -> Self {
+            Options {
+                standard: true,
+                legacy: true,
+                packed: true,
+                named: true,
+            }
+        }
+    }
+
+    impl Options {
+        fn no_standard(self) -> Self {
+            Options {
+                standard: false,
+                ..self
+            }
+        }
+
+        fn no_legacy(self) -> Self {
+            Options {
+                legacy: false,
+                ..self
+            }
+        }
+
+        fn no_packed(self) -> Self {
+            Options {
+                packed: false,
+                ..self
+            }
+        }
+
+        fn no_named(self) -> Self {
+            Options {
+                named: false,
+                ..self
+            }
+        }
+    }
+
+    fn from_slice_stream_options<'a, T>(
+        slice: &'a [u8],
+        options: Options,
     ) -> error::Result<(&'a [u8], T)>
     where
         T: serde_de::Deserialize<'a>,
     {
         let deserializer = Deserializer::from_slice(slice);
-        let deserializer = if !standard {
+        let deserializer = if !options.packed {
+            deserializer.disable_packed_format()
+        } else {
+            deserializer
+        };
+        let deserializer = if !options.named {
+            deserializer.disable_named_format()
+        } else {
+            deserializer
+        };
+        let deserializer = if !options.standard {
             deserializer.disable_standard_enums()
         } else {
             deserializer
         };
-        let mut deserializer = if !legacy {
+        let mut deserializer = if !options.legacy {
             deserializer.disable_legacy_enums()
         } else {
             deserializer
@@ -449,14 +507,17 @@ mod std_tests {
         ];
         let (_rest, value): (&[u8], Enum) = from_slice_stream(&v[..]).unwrap();
         assert_eq!(value, Enum::NewType(10));
-        let (_rest, value): (&[u8], Enum) = from_slice_stream_options(&v[..], true, false).unwrap();
+        let (_rest, value): (&[u8], Enum) =
+            from_slice_stream_options(&v[..], Options::default().no_legacy()).unwrap();
         assert_eq!(value, Enum::NewType(10));
-        let value: error::Result<(&[u8], Enum)> = from_slice_stream_options(&v[..], false, true);
+        let value: error::Result<(&[u8], Enum)> =
+            from_slice_stream_options(&v[..], Options::default().no_standard());
         assert_eq!(
             value.unwrap_err().classify(),
             serde_cbor::error::Category::Syntax
         );
-        let value: error::Result<(&[u8], Enum)> = from_slice_stream_options(&v[..], false, false);
+        let value: error::Result<(&[u8], Enum)> =
+            from_slice_stream_options(&v[..], Options::default().no_standard().no_legacy());
         assert_eq!(
             value.unwrap_err().classify(),
             serde_cbor::error::Category::Syntax
@@ -467,11 +528,14 @@ mod std_tests {
         ];
         let (_rest, value): (&[u8], Enum) = from_slice_stream(&v[..]).unwrap();
         assert_eq!(value, Enum::Unit);
-        let (_rest, value): (&[u8], Enum) = from_slice_stream_options(&v[..], true, false).unwrap();
+        let (_rest, value): (&[u8], Enum) =
+            from_slice_stream_options(&v[..], Options::default().no_legacy()).unwrap();
         assert_eq!(value, Enum::Unit);
-        let (_rest, value): (&[u8], Enum) = from_slice_stream_options(&v[..], false, true).unwrap();
+        let (_rest, value): (&[u8], Enum) =
+            from_slice_stream_options(&v[..], Options::default().no_standard()).unwrap();
         assert_eq!(value, Enum::Unit);
-        let value: error::Result<(&[u8], Enum)> = from_slice_stream_options(&v[..], false, false);
+        let value: error::Result<(&[u8], Enum)> =
+            from_slice_stream_options(&v[..], Options::default().no_legacy().no_standard());
         assert_eq!(
             value.unwrap_err().classify(),
             serde_cbor::error::Category::Syntax
@@ -486,14 +550,162 @@ mod std_tests {
         ];
         let (_rest, value): (&[u8], Enum) = from_slice_stream(&v[..]).unwrap();
         assert_eq!(value, Enum::NewType(10));
-        let value: error::Result<(&[u8], Enum)> = from_slice_stream_options(&v[..], true, false);
+        let value: error::Result<(&[u8], Enum)> =
+            from_slice_stream_options(&v[..], Options::default().no_legacy());
         assert_eq!(
             value.unwrap_err().classify(),
             serde_cbor::error::Category::Syntax
         );
-        let value: error::Result<(&[u8], Enum)> = from_slice_stream_options(&v[..], false, true);
+        let value: error::Result<(&[u8], Enum)> =
+            from_slice_stream_options(&v[..], Options::default().no_standard());
         assert_eq!(value.unwrap().1, Enum::NewType(10));
-        let value: error::Result<(&[u8], Enum)> = from_slice_stream_options(&v[..], false, false);
+        let value: error::Result<(&[u8], Enum)> =
+            from_slice_stream_options(&v[..], Options::default().no_standard().no_legacy());
+        assert_eq!(
+            value.unwrap_err().classify(),
+            serde_cbor::error::Category::Syntax
+        );
+    }
+
+    #[test]
+    fn test_packed_deserialization() {
+        #[derive(Debug, PartialEq, Deserialize)]
+        struct User {
+            user_id: u32,
+            password_hash: [u8; 4],
+        }
+
+        // unpacked
+        let v: Vec<u8> = vec![
+            0xa2, // map 2pair
+            0x67, 0x75, 0x73, 0x65, 0x72, 0x5f, 0x69, 0x64, // utf8 string: user_id
+            0x0a, // integer: 10
+            // utf8 string: password_hash
+            0x6d, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x5f, 0x68, 0x61, 0x73, 0x68,
+            0x84, 0x01, 0x02, 0x03, 0x04, // 4 byte array [1, 2, 3, 4]
+        ];
+
+        let (_rest, value): (&[u8], User) = from_slice_stream(&v[..]).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let (_rest, value): (&[u8], User) =
+            from_slice_stream_options(&v[..], Options::default().no_packed()).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let value: error::Result<(&[u8], User)> =
+            from_slice_stream_options(&v[..], Options::default().no_named());
+        assert_eq!(
+            value.unwrap_err().classify(),
+            serde_cbor::error::Category::Syntax
+        );
+
+        // unpacked - indefinite length
+        let v: Vec<u8> = vec![
+            0xbf, // map to be followed by a break
+            0x67, 0x75, 0x73, 0x65, 0x72, 0x5f, 0x69, 0x64, // utf8 string: user_id
+            0x0a, // integer: 10
+            // utf8 string: password_hash
+            0x6d, 0x70, 0x61, 0x73, 0x73, 0x77, 0x6f, 0x72, 0x64, 0x5f, 0x68, 0x61, 0x73, 0x68,
+            0x84, 0x01, 0x02, 0x03, 0x04, // 4 byte array [1, 2, 3, 4]
+            0xff, // break
+        ];
+
+        let (_rest, value): (&[u8], User) = from_slice_stream(&v[..]).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let (_rest, value): (&[u8], User) =
+            from_slice_stream_options(&v[..], Options::default().no_packed()).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let value: error::Result<(&[u8], User)> =
+            from_slice_stream_options(&v[..], Options::default().no_named());
+        assert_eq!(
+            value.unwrap_err().classify(),
+            serde_cbor::error::Category::Syntax
+        );
+
+        // packed
+        let v: Vec<u8> = vec![
+            0xa2, // map 2pair
+            0x00, // index 0
+            0x0a, // integer: 10
+            0x01, // index 1
+            0x84, 0x01, 0x02, 0x03, 0x04, // 4 byte array [1, 2, 3, 4]
+        ];
+
+        let (_rest, value): (&[u8], User) = from_slice_stream(&v[..]).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let (_rest, value): (&[u8], User) =
+            from_slice_stream_options(&v[..], Options::default().no_named()).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let value: error::Result<(&[u8], User)> =
+            from_slice_stream_options(&v[..], Options::default().no_packed());
+        assert_eq!(
+            value.unwrap_err().classify(),
+            serde_cbor::error::Category::Syntax
+        );
+
+        // packed - indefinite length
+        let v: Vec<u8> = vec![
+            0xbf, // map, to be followed by a break
+            0x00, // index 0
+            0x0a, // integer: 10
+            0x01, // index 1
+            0x84, 0x01, 0x02, 0x03, 0x04, // 4 byte array [1, 2, 3, 4]
+            0xff, // break
+        ];
+
+        let (_rest, value): (&[u8], User) = from_slice_stream(&v[..]).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let (_rest, value): (&[u8], User) =
+            from_slice_stream_options(&v[..], Options::default().no_named()).unwrap();
+        assert_eq!(
+            value,
+            User {
+                user_id: 10,
+                password_hash: [1, 2, 3, 4],
+            }
+        );
+        let value: error::Result<(&[u8], User)> =
+            from_slice_stream_options(&v[..], Options::default().no_packed());
         assert_eq!(
             value.unwrap_err().classify(),
             serde_cbor::error::Category::Syntax