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

serde untagged enums are broken #203

Closed
benkay86 opened this issue Mar 22, 2020 · 9 comments
Closed

serde untagged enums are broken #203

benkay86 opened this issue Mar 22, 2020 · 9 comments
Labels
bug serde Issues related to mapping from Rust types to XML

Comments

@benkay86
Copy link

benkay86 commented Mar 22, 2020

quick-xml is an great crate, and being able to use it with serde would be just amazing! It looks like there are a few issues already open with regard to buggy serde support, namely #185 and #190. This issue is probably related.

Unless I have misunderstood how enums are supposed to work, serde support for deserializing untagged enums appears to be broken:

#[derive(Debug, serde::Deserialize)]
#[serde(untagged)]
enum Y {
    AB{a: String, b: String},
    C{c: String}
}

#[derive(Debug, serde::Deserialize)]
struct X {
    y: Y,
}

fn main() {
    let xml_data = r#"
        <x>
            <y>
                <a>The letter a.</a>
                <b>The letter b.</b>
            </y>
        </x>
    "#;
    let x: X = quick_xml::de::from_str(xml_data).unwrap();
    println!("{:?}", x);
}
Custom("data did not match any variant of untagged enum Y")

In fairness, this does not work in serde-xml-rs either.

For others who may be reading this issue looking for a quick solution, I am using the following ugly hack as a workaround. Improvements are welcome.

#[derive(Debug, serde::Deserialize)]
struct YRaw {
    a: Option<String>,
    b: Option<String>,
    c: Option<String>,
}
#[derive(Debug, serde::Deserialize)]
#[serde(try_from = "YRaw")]
enum Y {
    #[allow(unused)]
    AB{a: String, b: String},
    #[allow(unused)]
    C{c: String}
}
impl std::convert::TryFrom<YRaw> for Y {
    type Error = &'static str;
    fn try_from(yraw: YRaw) -> Result<Self, Self::Error> {
        if yraw.a.is_some() && yraw.b.is_some() && yraw.c.is_some() {
            return Err("Y cannot be both AB and C.");
        }
        if let Some(c) = yraw.c {
            return Ok(Self::C{c});
        }
        if let (Some(a), Some(b)) = (yraw.a, yraw.b) {
            return Ok(Self::AB{a,b});
        }
        Err("Y is not AB or C.")
    }
}
#[derive(Debug, serde::Deserialize)]
struct X {
    y: Y,
}
@carrascomj
Copy link

As of now, it is working for me with Deserialize (here) but it remains incapable to Serialize, it omits the untagged field on write.

@tafia
Copy link
Owner

tafia commented Nov 14, 2020

Thanks for commenting it!

@carrascomj
Copy link

carrascomj commented Nov 14, 2020

It turned out that my untagged enum was being serialized, just not as an attribute (as I wanted). I think this is the expected behavior, so this is solved from my side.

Enum as attribute

I am not sure if there is something more straight forward that I'm missing, but this solved the enum as attribute problem for me.
I needed Root to be serialized as

<root kind="calamari" sense="whatever"/>

Structs and enums definitions:

use serde::{Deserialize, Serialize, Serializer};

#[derive(Deserialize, Serialize)]
struct Root {
    kind: A,
    sense: String,
}

// do not derive `Serialize`!
#[derive(Deserialize)]
#[serde(untagged)]
enum A {
    NestedEnum(B),
    Other(String)
}

#[derive(Deserialize, Serialize)]
#[serde(rename_all="camelCase")]
enum B {
     Calamari,
     Tortoise,
     Octopus
}

Serialize implementation (for the nested enum, a macro can be used like this one):

impl Serialize for A {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
    where
        S: Serializer,
    {
        match self {
            Self::NestedEnum(ref b) => serializer.serialize_str(match b {
                  B::Calamari => "calamari",  // in lower case for my specific case
                  B::Tortoise => "tortoise",
                  B::Octopus => "octopus",
           }
),
            Self::Other(s) => serializer.serialize_str(s),
        }
    }
}

@benkay86
Copy link
Author

The sample code at the top of this issue still fails to match the Y::AB variant and panics, even when testing against quick-xml from git master. @carrascomj, it looks like you got some version of untagged enums to deserialize in your code. Could you please share a minimal working example program, including the xml that was successfully deserialized?

@MoSal
Copy link

MoSal commented May 30, 2021

Attributes seem to deserialize successfully. Elements fail!

@MoSal
Copy link

MoSal commented May 30, 2021

Actually, OP's example succeeds with $value-wrapping structs A and B.

I'll have to investigate further why my code is hitting this error.

@MoSal
Copy link

MoSal commented May 30, 2021

Found my issue. It's related to vector, or optional vector fields:

use serde::Deserialize;

fn main() {
    let bs = br###"
<Xs x_id="bla">
  <st>
    <v id="some_id">some_s</v>
  </st>
</Xs>
    "###;

    // works as expected with 0, 1, or more `v` elements
    let xs: Xs = quick_xml::de::from_reader(&bs[..]).unwrap();
    eprintln!("{:#?}", xs);

    let bn = br###"
<Xn x_id="bla">
  <en>
    <v id="some_id">some_s</v>
  </en>
</Xn>
    "###;
    // fails with 1 or more `v` elements
    let xn: Xn = quick_xml::de::from_reader(&bn[..]).unwrap();
    eprintln!("{:#?}", xn);
}

#[derive(Deserialize, Debug)]
#[serde(deny_unknown_fields)] // TODO: remove after some tests
struct SWithId {
    id: String,
    #[serde(rename="$value")]
    s: String,
}

#[derive(Deserialize, Debug)]
#[serde(deny_unknown_fields)] // TODO: remove after some tests
#[serde(untagged)]
enum En {
    S{
        s: String,
    },
    V{
        //v: Option<SWithId>, // works
        //v: Vec<SWithId>, // fails
        v: Option<Vec<SWithId>>, // fails
    },
}

#[derive(Deserialize, Debug)]
#[serde(deny_unknown_fields)] // TODO: remove after some tests
struct St {
    // v: Vec<SWithId>, // works
    v: Option<Vec<SWithId>>, // works
}

#[derive(Deserialize, Debug)]
#[serde(deny_unknown_fields)]
pub struct Xn {
    x_id: String,
    en: Option<En>,
}

#[derive(Deserialize, Debug)]
#[serde(deny_unknown_fields)]
pub struct Xs {
    x_id: String,
    st: Option<St>,
}

Should I create a new issue?

@Mingun Mingun added bug serde Issues related to mapping from Rust types to XML labels May 21, 2022
@Mingun
Copy link
Collaborator

Mingun commented Sep 7, 2022

@MoSal, your last problem is the serde restriction due to serde-rs/serde#1183. serde first uses generic Content type to deserialize XML into it and then tryes to deserialize each variant of untagged enum from it. It uses deserialize_any which ask from the data format what type should be deserialized.

<Xn x_id="bla">
  <en>
    <v id="some_id">some_s</v>
  </en>
</Xn>

generically deserialized into

Content::Map {
  Content::String("x_id") => Content::String("bla"),
  Content::String("en") => Content::Map {
    Content::String("v") => Content::Map {
      Content::String("id") => Content::String("some_id"),
      Content::String("$value") => Content::String("some_s"),
    },
  },
}
  • Then it tries to deserialize En::S from Content::Map for key Content::String("en"), which is failed because of absence of s field in the map.
  • Then it tries to deserialize En::V from Content::Map for key Content::String("en"):
    • Then it tries to deserialize Vec from Content::Map for key Content::String("v"), which is impossible.
  • In the end no variant would be deserialized and data did not match any variant of untagged enum En error is returned.

In XML we cannot distinguish between an array and a single element when there is only one element, because arrays has no special syntax in XML. So your problem is unsolvable using that types. Avoid using untagged enums, internally tagged enums, #[serde(flatten)] and any types that uses deserialize_any. They will not work

@Mingun
Copy link
Collaborator

Mingun commented Sep 7, 2022

@benkay86 , your case not work for the same reason, but more tricky. While

#[derive(Debug, Deserialize, PartialEq)]
struct AB {
    a: String,
    b: String,
}
#[derive(Debug, Deserialize, PartialEq)]
struct X2 {
    y: AB,
}

let xml_data = r#"
    <x>
        <y>
            <a>The letter a.</a>
            <b>The letter b.</b>
        </y>
    </x>
"#;

let x: X2 = quick_xml::de::from_str(xml_data).unwrap();
assert_eq!(x, X2 { y: AB {
    a: "The letter a.".to_string(),
    b: "The letter b.".to_string(),
}});

works fine, untagged enums will not work due to serde-rs/serde#1183.

  • First, XML buffered into
    Content::Map {
      Content::String("y") => Content::Map {
        Content::String("a") => Content::Map {
          Content::String("$value") => Content::String("The letter a."),
        },
        Content::String("b") => Content::Map {
          Content::String("$value") => Content::String("The letter b."),
        },
      },
    }
  • Second, it tries to deserialize String a from
    Content::Map {
      Content::String("$value") => Content::String("The letter a."),
    }
    which is impossible
  • Variant Y::C obviously cannot be deserialized from that XML
  • In the end no variant would be deserialized and data did not match any variant of untagged enum Y error is returned.

I close this, because we cannot do anything with that at quick-xml level.

@Mingun Mingun closed this as not planned Won't fix, can't repro, duplicate, stale Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug serde Issues related to mapping from Rust types to XML
Projects
None yet
Development

No branches or pull requests

5 participants