fix | Implement de_ignored_any to consume unknown fields #5
Conversation
frunobulax-the-poodle
changed the title
There was a problem hiding this comment.
Thank you for the pull request.
However I believe that ignored_any should completely ignore the value and end on a visit_unit. For example that is what serde_json does: https://docs.rs/serde_json/latest/src/serde_json/de.rs.html#1897-1903
| 0 => self.deserialize_u32(visitor), | ||
| 1 => self.deserialize_i32(visitor), |
There was a problem hiding this comment.
This means that a value that does not fit in a i32 or a u32 de-serialization will fail.
I believe this should not actually de-serialize the value but just discard it.
| struct Struct { | ||
| a: u8, | ||
| b: i16, | ||
| } | ||
|
|
||
| #[derive(Debug, Serialize)] | ||
| struct SuperStruct { | ||
| a: u8, | ||
| c: Struct, | ||
| b: i16, | ||
| } | ||
|
|
||
| let mut buf = [0u8; 64]; | ||
| let base = Struct { a: 22, b: -161 }; | ||
| let sup = SuperStruct { a: 22, b: -161, c: base.clone() }; | ||
|
|
||
| let ser = cbor_serialize(&sup, &mut buf).unwrap(); | ||
| let de: Struct = cbor_deserialize(&ser).unwrap(); | ||
|
|
||
| assert_eq!(de, base); | ||
| } |
There was a problem hiding this comment.
Here if you replace the u8 with u64 and 22 with 0xFFFFFFFFF this will lead to DeserializeBadU32.
Seems you're right. This was pretty far outside my wheelhouse so I expected to screw something up :) As you've already got a better PR out and are in a better position to maybe actually get it merged I'd be happy to just close this one. |
|
I will close this PR in favour of #6 Now that this has been moved to Thank you. |
Short Context
The MicroG implementation of FIDO2 includes the optional transports which isn't in ctap-types and the "appid" extension.
After I day I tracked the issue down to the cbor parser, which fails on map fields not present in the struct it's trying to parse.
Issue
When deserializing a map that has fields not present in the struct, the key is consumed, but the value is instead skipped entirely.
When deserialization continues, there is leftover unexpected data in the input, leading to errors (usually bad major)
Fix
Implement
deserialize_ignored_anyforDeserializer, so the values are fully consumed.This does kind of go against not supporting
deserialize_any, but I don't think there's a way around it.