media-io / yaserde

Yet Another Serializer/Deserializer
MIT License
174 stars 58 forks source link

Yaserde gets stuck in a loop during deserialization of an enum #146

Closed lifichi closed 1 year ago

lifichi commented 1 year ago

Hello,

During deserialization of a XML element as enum variant (see example below), yaserde gets stuck in a loop.

use yaserde;
use yaserde_derive::{YaSerialize, YaDeserialize};
use simple_log;

const XML: &str = r#"<Device><notKnown/></Device>"#;

#[derive(YaSerialize, YaDeserialize, Default)]
pub enum Device {
    #[default] notKnown,
    known,
}

fn main() {
    simple_log::quick!("trace");
    let dev: Device = yaserde::de::from_str(&XML).unwrap();
}

Log emitted by yaserde:

[DEBUG] <csta_looping:7>:Enum Device @ 0: start to parse "Device"
[TRACE] <csta_looping:7>:Enum Device @ 0: matching StartElement(Device, {"": "", "xml": "http://www.w3.org/XML/1998/namespace", "xmlns": "http://www.w3.org/2000/xmlns/"})
[DEBUG] <yaserde::de:88>:Fetched StartElement(Device, {"": "", "xml": "http://www.w3.org/XML/1998/namespace", "xmlns": "http://www.w3.org/2000/xmlns/"}), new depth 1
[TRACE] <csta_looping:7>:Enum Device @ 0: matching StartElement(notKnown, {"": "", "xml": "http://www.w3.org/XML/1998/namespace", "xmlns": "http://www.w3.org/2000/xmlns/"})
[TRACE] <csta_looping:7>:Enum Device @ 0: matching StartElement(notKnown, {"": "", "xml": "http://www.w3.org/XML/1998/namespace", "xmlns": "http://www.w3.org/2000/xmlns/"})
[TRACE] <csta_looping:7>:Enum Device @ 0: matching StartElement(notKnown, {"": "", "xml": "http://www.w3.org/XML/1998/namespace", "xmlns": "http://www.w3.org/2000/xmlns/"})
and so on, over and over ...

EDIT: After some examination I found that in file yaserde_derive/src/de/expand_enum.rs in lines from 112 to 117 during code generation for a unit variant, there is no a break; statment for a match arm

Fields::Unit => Some(quote! {
  #xml_element_name => {
    enum_value = ::std::option::Option::Some(#variant_name);
  }
}),

so generated code looks like this:

loop {
    let event = reader.peek()?.to_owned();
    /* logging code was removed from here */
    match event {
        ::yaserde::__xml::reader::XmlEvent::StartElement {
            ref name,
            ref attributes,
            ..
        } => {
            match name.local_name.as_str() {
                // one of these arms are matched ("noKnown" or "known")
                // but we don't break from the loop...
                "notKnown" => {
                    enum_value = ::std::option::Option::Some(Device::notKnown);
                }
                "known" => {
                    enum_value = ::std::option::Option::Some(Device::known);
                }
                _named_element => {
                    let _root = reader.next_event();
                }
            }
            // ... and we don't advance the reader (reader.peek()?.to_owned() is still XmlEvent::StartElement)
            // so this `if` block doesn't execute...
            if let ::yaserde::__xml::reader::XmlEvent::Characters(content) =
                reader.peek()?.to_owned()
            {
                match content.as_str() {
                    "notKnown" => {
                        enum_value = ::std::option::Option::Some(Device::notKnown);
                    }
                    "known" => {
                        enum_value = ::std::option::Option::Some(Device::known);
                    }
                    _ => {}
                }
            }
            // .. so we loop again
        }
        // arms that don't match...
    }
    // end of a loop block
}

Adding break; statement after the match "solves" this problem and the example from the beginning works as intended:

Fields::Unit => Some(quote! {
  #xml_element_name => {
    enum_value = ::std::option::Option::Some(#variant_name);
    break;
  }
}),

but I don't known if this is the correct solution, I don't know if this might break some other code...