tafia / quick-xml

Rust high performance xml reader and writer
MIT License
1.19k stars 234 forks source link

Vec<String> inside of struct serializes incorrectly with serde #280

Closed dralley closed 1 year ago

dralley commented 3 years ago

I was attempting to add quick-xml to the Rust serializer benchmarks and discovered that one of the complex benchmarks serialized incorrectly, resulting in errors on deserialization. The benchmark in question is the "minecraft savedata" benchmark.

I minimized it so it's less unwieldy. Here's a test case.


const RECIPES: [&'static str; 8] = [
    "pickaxe",
    "torch",
    "bow",
    "crafting table",
    "furnace",
    "shears",
    "arrow",
    "tnt",
];

#[derive(serde::Deserialize, serde::Serialize)]
pub struct RecipeBook {
    pub recipes: Vec<String>,
}

#[test]
fn test_fail_serialize() {
    let recipe_book = RecipeBook {
        recipes: RECIPES.iter().map(|s| s.to_string()).collect(),
    };

    let serialized = to_string(&recipe_book).unwrap();

    println!("{}", serialized);
    let deserialized: RecipeBook = from_str(&serialized).unwrap();
}

Prints the following when tests are run:

---- test_fail_serialize stdout ----
<RecipeBook recipes="pickaxetorchbowcrafting tablefurnaceshearsarrowtnt"/>
thread 'test_fail_serialize' panicked at 'called `Result::unwrap()` on an `Err` value: Custom("invalid type: string \"pickaxetorchbowcrafting tablefurnaceshearsarrowtnt\", expected a sequence")', tests/serde_roundtrip.rs:55:58
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The problem is that recipes: Vec<String> and to_be_displayed: Vec<String> are being serialized into one long concatenated string and then assigned to an attribute of a self-closing element, rather than being serialized to a list of inner elements. Deserialize does the correct thing and expects a list of strings.

dralley commented 3 years ago

This may be related to https://github.com/tafia/quick-xml/pull/205

shalinnijel2 commented 2 years ago

Is there a potential work around for this issue while this is still being worked on?

shalinnijel2 commented 2 years ago

I guess using a custom serializer which uses serialize_field() is one option...

zmrow commented 2 years ago

I'm also wondering what the workaround is here... :)

Mingun commented 2 years ago

Workaround is to use a Newtype idiom:

#[test]
fn test_fail_serialize() {
    const RECIPES: [&'static str; 2] = [
        "pickaxe",
        "crafting table",
    ];

    #[derive(Debug, PartialEq, serde::Deserialize, serde::Serialize)]
    struct Workaround(String);

    #[derive(Debug, PartialEq, serde::Deserialize, serde::Serialize)]
    struct RecipeBook {
        recipes: Vec<Workaround>,
    }

    let recipe_book = RecipeBook {
        recipes: RECIPES.iter().map(|s| Workaround(s.to_string())).collect(),
    };

    let serialized = quick_xml::se::to_string(&recipe_book).unwrap();

    assert_eq!(serialized, "<RecipeBook><recipes>pickaxe</recipes><recipes>crafting table</recipes></RecipeBook>");

    let deserialized: RecipeBook = from_str(&serialized).unwrap();
    assert_eq!(deserialized, RecipeBook {
        recipes: RECIPES.iter().map(|s| Workaround(s.to_string())).collect(),
    });
}
zmrow commented 2 years ago

@Mingun Thanks so much!! :tada: