Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks done
alexjg opened this issue Jan 21, 2023 · 10 comments
Open
2 tasks done
Assignees
Labels
bug Something isn't working

Comments

@alexjg
Copy link

alexjg commented Jan 21, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Code of Conduct

  • 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 alexjg added the bug Something isn't working label Jan 21, 2023
@alexjg alexjg changed the title [Bug]: Unable to deserialize internall tagged enum containing semantically tagged CBOR [Bug]: Unable to deserialize internally tagged enum containing semantically tagged CBOR Jan 21, 2023
@alexjg
Copy link
Author

alexjg commented Jan 21, 2023

I believe this error is triggered by this line:

visitor.visit_enum(access)

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.

@benwis
Copy link

benwis commented May 21, 2023

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

@rjzak
Copy link
Member

rjzak commented May 21, 2023

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

@benwis
Copy link

benwis commented May 21, 2023

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

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

@rjzak rjzak self-assigned this May 22, 2023
@rjzak
Copy link
Member

rjzak commented May 29, 2023

@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?

@alexjg
Copy link
Author

alexjg commented May 29, 2023

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.

@benwis
Copy link

benwis commented May 29, 2023

@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

@rjzak
Copy link
Member

rjzak commented May 29, 2023

@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.

@shimunn
Copy link

shimunn commented Feb 1, 2024

Is there any progress or workaround ?

@rjzak
Copy link
Member

rjzak commented Feb 2, 2024

No unfortunately. See my prior comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: New
Development

No branches or pull requests

4 participants