netvl / xml-rs

An XML library in Rust
MIT License
461 stars 113 forks source link

Version 0.8.9 broke deserialization behavior #223

Closed jonasbb closed 1 year ago

jonasbb commented 1 year ago

I noticed a failing test in my CI. I am using xml-rs via serde-xml-rs. The breaking change occurred between v0.8.8 and v0.8.9. The problem also occurs in v0.8.10.

main.rs

fn main() {
    use serde::Deserialize;

    #[derive(Debug, PartialEq, Deserialize)]
    #[serde(rename_all = "lowercase")]
    enum Item {
        Foo(String),
    }

    // Deserialize this XML
    let items: Vec<Item> = serde_xml_rs::from_str(
        r"
<foo>a</foo>
<foo>b</foo>
",
    )
    .unwrap();

    let expected = vec![
        Item::Foo(String::from("a")),
        Item::Foo(String::from("b")),
    ];
    assert_eq!(expected, items);
}

Cargo.toml

[package]
name = "xml-rs-bug"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
serde = { version = "1.0.163", features = ["derive"] }
serde-xml-rs = "0.6.0"
xml-rs = "=0.8.9"
# The code runs fine with this version of xml-rs
# xml-rs = "=0.8.8"

One possible commit could be this https://github.com/netvl/xml-rs/commit/ae9d83b3aec66ed6d8ac9dd5746d33d523c858cc since it has something to do with multiple root elements.

kornelski commented 1 year ago

This isn't really related to deserialization specifically. The markup you're using is not a proper XML — well-formed XML always only allows one root element. If you have multiple elements, they all must be descendants of a single root element.

So you were relying on a what I consider a bug. For backwards compatibility I've kept old behaviour as an option — you need to enable allow_multiple_root_elements. Or fix the XML.

kornelski commented 1 year ago

I've changed default config in 0.8.11 to allow ill-formed roots again.