ciborium icon indicating copy to clipboard operation
ciborium copied to clipboard

[Bug]: Unable to deserialize internally tagged enum containing semantically tagged CBOR

Open alexjg opened this issue 2 years ago • 10 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

Current Behaviour

The following program crashes:

use ciborium::cbor;

fn main() {
    let data = cbor!({
        "type" => "thing2",
        "name" => "somename",
        "data" => ciborium::tag::Required::<Vec<u8>, 64>(vec![1, 2, 3, 4]),
    }).unwrap();
    let mut bytes = Vec::new();
    ciborium::ser::into_writer(&data, &mut bytes).unwrap();

    let _thing: DistinctThings = ciborium::de::from_reader(bytes.as_slice()).unwrap();
}

#[derive(serde::Deserialize)]
#[serde(tag = "type")]
enum DistinctThings {
    #[serde(rename = "thing1")]
    Thing1 {
        #[serde(rename = "name")]  
        some_name: String,
    },
    #[serde(rename = "thing2")]
    Thing2 {
        #[serde(rename = "name")]  
        some_name: String,
    }
}

With the error:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Semantic(None, "untagged and internally tagged enums do not support enum input")', src/main.rs:12:78
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expected Behaviour

The DistinctThings enum should be succesfully deserialized.

Environment Information

Linux manjaro-desktop 5.15.85-1-MANJARO #1 SMP PREEMPT Wed Dec 21 21:15:06 UTC 2022 x86_64 GNU/Linux

Steps To Reproduce

No response

alexjg avatar Jan 21 '23 10:01 alexjg

I believe this error is triggered by this line: https://github.com/enarx/ciborium/blob/a6aeb1697671eabe6503626872b732ccb69940c0/ciborium/src/de/mod.rs#L180

This is triggered because serde has to traverse the cbor map using serde::private::de::TaggedContentVisitor to collect the tag and content before constructing the variant. In order to do this serde deserializes each key in the map (this is in serde::private::de::TaggedContentVisitor::visit_map), calling MapAccess::next_value for each value, this forwards (via <ciborium::de::Acces as MapAccess>::next_value_seed) to ciborium::Deserializer::deserialize_any. At this point the next value in the stream is the semantically tagged item, so we match Header::Tag and then we end up calling visitor.visit_enum. However, the visitor at this point is a serde::private::de::ContentVisitor (the visitor serde uses for fields in the map which are not the tag field) and ContentVisitor does not allow visit_enum for some reason.

alexjg avatar Jan 21 '23 11:01 alexjg

This is a nasty bug, and is basically requiring us to switch off ciborium to serde_cbor to represent this internally tagged API we're working with. Hoping to get this fixed

benwis avatar May 21 '23 16:05 benwis

I'll take a look at this, hopefully in the coming days.

rjzak avatar May 21 '23 17:05 rjzak

I'll take a look at this, hopefully in the coming days.

Let me know if there's anything I can provide to help!

benwis avatar May 21 '23 17:05 benwis

@alexjg it seems the error comes from serde itself as you suggested.

https://github.com/serde-rs/serde/blob/ee9166ec97074c4752e4f75dd846b1dfaac227d4/serde/src/private/de.rs#L495-L502

I'm not familiar with Serde internals.

@benwis Is it too much to ask for a PR?

rjzak avatar May 29 '23 14:05 rjzak

I think the basic problem is using an enum (tag::Internal) to pass around the tag state. The problem with this is that if we are deserializing a tagged enum we end up asking the visitor to visit an enum (the tag::Internal enum) whilst inside an enum variant (of the user defined tagged enum). I suspect that just changing the representation of tag::Internal to something which is allowed inside an enum variant such as a two tuple would do the trick.

alexjg avatar May 29 '23 16:05 alexjg

@alexjg it seems the error comes from serde itself as you suggested.

https://github.com/serde-rs/serde/blob/ee9166ec97074c4752e4f75dd846b1dfaac227d4/serde/src/private/de.rs#L495-L502

I'm not familiar with Serde internals.

@benwis Is it too much to ask for a PR?

I'm not sure I know enough about this to provide a PR, but I can provide a minimally reproducible example if that's helpful

benwis avatar May 29 '23 21:05 benwis

@benwis the original example in the issue is enough to trigger the problem. I don't have the experience to figure this out any time soon, so I was hoping there would be some assistance in the form of a PR, even if incomplete that could get me going in the right direction.

rjzak avatar May 29 '23 21:05 rjzak

Is there any progress or workaround ?

shimunn avatar Feb 01 '24 16:02 shimunn

No unfortunately. See my prior comment.

rjzak avatar Feb 02 '24 02:02 rjzak