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

UnexpectedEof error on 0.28. #580

Closed
BratSinot opened this issue Mar 20, 2023 · 6 comments · Fixed by #660
Closed

UnexpectedEof error on 0.28. #580

BratSinot opened this issue Mar 20, 2023 · 6 comments · Fixed by #660
Labels
bug serde Issues related to mapping from Rust types to XML

Comments

@BratSinot
Copy link

BratSinot commented Mar 20, 2023

Greeting!

Start getting error on 0.28 version (0.27 work fine).

Here example to reproduce error:
P.S. XmlParser trait needed because I need also json serialization / deserialization.

#[derive(Debug, Deserialize)]
pub struct Foo {
    #[serde(default, rename = "newIDs")]
    pub new_ids: Vec<NewIds>,
}

#[derive(Debug, Deserialize)]
pub struct NewIds {
    #[serde(rename = "$value", deserialize_with = "XmlParser::parse")]
    pub objects: Vec<Bar>,
}

#[derive(Debug)]
pub struct Bar {
    pub id: u32,
    pub num: u32,
}

pub trait XmlParser {
    fn parse<'de, D>(deserializer: D) -> Result<Self, D::Error>
    where
        Self: Sized,
        D: Deserializer<'de>;
}

impl XmlParser for Bar {
    fn parse<'de, D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        #[allow(clippy::enum_variant_names)]
        #[derive(Copy, Clone, Deserialize)]
        enum _Bar {
            One(NewId),
        }

        #[derive(Copy, Clone, Deserialize)]
        struct NewId {
            #[serde(rename = "@newID")]
            id: u32,
        }

        let (id, num) = match _Bar::deserialize(deserializer)? {
            _Bar::One(NewId { id }) => (id, 1),
        };

        Ok(Bar { id, num })
    }
}

impl<T> XmlParser for Vec<T>
where
    T: XmlParser,
{
    fn parse<'de, D>(deserializer: D) -> Result<Self, D::Error>
    where
        Self: Sized,
        D: Deserializer<'de>,
    {
        #[derive(Deserialize)]
        struct _T<T: XmlParser>(#[serde(deserialize_with = "XmlParser::parse")] T);

        let arr = Vec::deserialize(deserializer)?
            .into_iter()
            .map(|_T(t)| t)
            .collect();

        Ok(arr)
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    const XML: &str = r#"<Foo>
            <newIDs>
                <One newID="1"    />
                <One newID="2"    />
            </newIDs>
        </Foo>"#;

    println!("{:?}", from_str::<Foo>(XML)?);

    Ok(())
}
@Mingun Mingun added bug serde Issues related to mapping from Rust types to XML labels Mar 20, 2023
@Mingun
Copy link
Collaborator

Mingun commented Mar 20, 2023

Thanks for report. I've seen the same error when investigating #567, so probably the cause is the same. It would be wonderful if you'll try to minify your example and do bisect between versions to find the exact commit where the bug was introduced. The history is mostly linear and mostly each commit contains working state, so this shouldn't be too hard.

The error is definitely related to sequence deserialization.

@BratSinot
Copy link
Author

It would be wonderful if you'll try to minify your example and do bisect between versions to find the exact commit where the bug was introduced.

It was started after 221b57d.
Previous (b3ebf7a) is OK.

@BratSinot
Copy link
Author

About simplifying, this is maximum what I can do:

[package]
name = "playground"
version = "0.1.0"
edition = "2021"

[dependencies]
#OK
#quick-xml = { git = "https://github.com/tafia/quick-xml.git", rev = "b3ebf7a038f3984803589eb19aaaf4e950bf68b3", features = ["serialize", "overlapped-lists"] }
#Not OK
quick-xml = { git = "https://github.com/tafia/quick-xml.git", rev = "221b57d73f9f44060bcf13f49e5868ee13ae1f55", features = ["serialize", "overlapped-lists"] }
use quick_xml::de::from_str;
use serde::{Deserialize, Deserializer};
use std::{error::Error, fmt::Debug};

trait XmlParser {
    fn parse<'de, D>(deserializer: D) -> Result<Self, D::Error>
    where
        Self: Sized,
        D: Deserializer<'de>;
}

#[derive(Debug, Deserialize)]
pub struct Seq {
    #[serde(rename = "$value", deserialize_with = "XmlParser::parse")]
    pub new_ids: Vec<Bar>,
}

#[derive(Debug)]
pub struct Bar {
    pub id: u32,
    pub num: u32,
}

impl XmlParser for Bar {
    fn parse<'de, D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: Deserializer<'de>,
    {
        #[allow(clippy::enum_variant_names)]
        #[derive(Copy, Clone, Deserialize)]
        enum _Bar {
            One(NewId),
            Two(NewId),
        }

        #[derive(Copy, Clone, Deserialize)]
        struct NewId {
            #[serde(rename = "@newID")]
            id: u32,
        }

        let (id, num) = match _Bar::deserialize(deserializer)? {
            _Bar::One(NewId { id }) => (id, 1),
            _Bar::Two(NewId { id }) => (id, 2),
        };

        Ok(Bar { id, num })
    }
}

// TODO: https://github.com/serde-rs/serde/issues/723
impl<T> XmlParser for Vec<T>
where
    T: XmlParser,
{
    fn parse<'de, D>(deserializer: D) -> Result<Self, D::Error>
    where
        Self: Sized,
        D: Deserializer<'de>,
    {
        #[derive(Deserialize)]
        struct _T<T: XmlParser>(#[serde(deserialize_with = "XmlParser::parse")] T);

        let arr = Vec::deserialize(deserializer)?
            .into_iter()
            .map(|_T(t)| t)
            .collect();

        Ok(arr)
    }
}

fn main() -> Result<(), Box<dyn Error>> {
    const XML: &str = r#"
        <Seq>
            <One newID="10" />
            <Two newID="20" />
        </Seq>"#;

    println!("{:?}", from_str::<Seq>(XML)?);

    Ok(())
}

@paramako
Copy link

any updates on this?

@BratSinot
Copy link
Author

any updates on this?

I just start to use different approach.
I have some XmlStruct which implement structure for quick-xml / serde without deserialize_with and OrdinaryStruct and convert xml to ordinary throught From trait.

@Mingun
Copy link
Collaborator

Mingun commented Oct 1, 2023

The minimal reproduction case is

/// Regression test for https://github.com/tafia/quick-xml/issues/580.
#[test]
fn issue580() {
    #[derive(Debug, Deserialize, PartialEq, Eq)]
    struct Seq {
        #[serde(rename = "$value")]
        items: Vec<Wrapper>,
    }

    #[derive(Debug, Deserialize, PartialEq, Eq)]
    struct Wrapper(#[serde(deserialize_with = "Item::parse")] Item);

    #[derive(Debug, PartialEq, Eq)]
    struct Item;
    impl Item {
        fn parse<'de, D>(_deserializer: D) -> Result<Self, D::Error>
        where
            D: serde::Deserializer<'de>,
        {
            Ok(Item)
        }
    }

    assert_eq!(
        from_str::<Seq>(
            r#"
        <Seq>
            <One/>
            <Two/>
        </Seq>"#
        )
        .unwrap(),
        Seq {
            items: vec![Wrapper(Item), Wrapper(Item)]
        }
    );
}

If replace vector item type from Wrapper to Item, the bug disappeared

Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 4, 2023
---- issue580 stdout ----
thread 'issue580' panicked at tests/serde-issues.rs:412:10:
called `Result::unwrap()` on an `Err` value: Custom("invalid length 0, expected tuple struct Wrapper with 1 element")
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    issue580
Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 4, 2023
…ct instead of visit_seq

The problem was in `SeqItemDeserializer::deserialize_newtype_struct`. Previously it was
the same as `SeqItemDeserializer::deserialize_seq`, but deserializing sequences in this
deserializer assumes, that those sequences are `xs:list`s, because deserializer itself
represents a list element.

In the original bug report an `UnexpectedEof` is returning. The error was changed in the
pre-previous commit (3d5ed69) and in this it is fixed.

The following tests checks that all's OK:

- Deserializer::deserialize_newtype_struct is reached by:
  - serde-de:
    - newtype::excess_attribute
    - newtype::simple
  - serde-se:
    - with_root::newtype
    - without_root::newtype

- MapValueDeserializer::deserialize_newtype_struct is reached by:
  - serde-issues:
    - issue343

- SeqItemDeserializer::deserialize_newtype_struct is reached by:
  - serde-issues:
    - issue580
Mingun added a commit to Mingun/quick-xml that referenced this issue Oct 7, 2023
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

Successfully merging a pull request may close this issue.

3 participants