Skip to content

Commit

Permalink
Do not report errors if deserializer cannot convert string to the boo…
Browse files Browse the repository at this point in the history
…lean or number

Call visit_borrowed_str / visit_str / visit_string instead. It is responsibility of
the type to return an error if it is not able to process such data; the deserializer
does the conversion only because technically all things in XML stored as strings
  • Loading branch information
Mingun committed Oct 20, 2024
1 parent b285fc9 commit 45a66e5
Show file tree
Hide file tree
Showing 8 changed files with 67 additions and 52 deletions.
3 changes: 3 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
`Vec<String>` in `$value` fields. They cannot be deserialized back with the same result
- [#827]: Make `escape` and it variants take a `impl Into<Cow<str>>` argument and implement
`From<(&'a str, Cow<'a, str>)>` on `Attribute`
- [#826]: Removed `DeError::InvalidInt`, `DeError::InvalidFloat` and `DeError::InvalidBoolean`.
Now the responsibility for returning the error lies with the visitor of the type.
See rationale in https://github.com/serde-rs/serde/pull/2811

[#227]: https://github.com/tafia/quick-xml/issues/227
[#655]: https://github.com/tafia/quick-xml/issues/655
Expand Down
8 changes: 5 additions & 3 deletions src/de/key.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::de::simple_type::UnitOnly;
use crate::de::str2bool;
use crate::encoding::Decoder;
use crate::errors::serialize::DeError;
use crate::name::QName;
Expand All @@ -14,7 +13,10 @@ macro_rules! deserialize_num {
where
V: Visitor<'de>,
{
visitor.$visit(self.name.parse()?)
match self.name.parse() {
Ok(number) => visitor.$visit(number),
Err(_) => self.name.deserialize_str(visitor),
}
}
};
}
Expand Down Expand Up @@ -139,7 +141,7 @@ impl<'de, 'd> Deserializer<'de> for QNameDeserializer<'de, 'd> {
where
V: Visitor<'de>,
{
str2bool(self.name.as_ref(), visitor)
self.name.deserialize_bool(visitor)
}

deserialize_num!(deserialize_i8, visit_i8);
Expand Down
3 changes: 2 additions & 1 deletion src/de/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ use crate::{
de::resolver::EntityResolver,
de::simple_type::SimpleTypeDeserializer,
de::text::TextDeserializer,
de::{str2bool, DeEvent, Deserializer, XmlRead, TEXT_KEY, VALUE_KEY},
de::{DeEvent, Deserializer, XmlRead, TEXT_KEY, VALUE_KEY},
encoding::Decoder,
errors::serialize::DeError,
errors::Error,
events::attributes::IterState,
events::BytesStart,
name::QName,
utils::CowRef,
};
use serde::de::value::BorrowedStrDeserializer;
use serde::de::{self, DeserializeSeed, Deserializer as _, MapAccess, SeqAccess, Visitor};
Expand Down
28 changes: 13 additions & 15 deletions src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1842,7 +1842,13 @@ macro_rules! deserialize_num {
{
// No need to unescape because valid integer representations cannot be escaped
let text = self.read_string()?;
visitor.$visit(text.parse()?)
match text.parse() {
Ok(number) => visitor.$visit(number),
Err(_) => match text {
Cow::Borrowed(t) => visitor.visit_str(t),
Cow::Owned(t) => visitor.visit_string(t),
}
}
}
};
}
Expand Down Expand Up @@ -1873,9 +1879,11 @@ macro_rules! deserialize_primitives {
where
V: Visitor<'de>,
{
let text = self.read_string()?;

str2bool(&text, visitor)
let text = match self.read_string()? {
Cow::Borrowed(s) => CowRef::Input(s),
Cow::Owned(s) => CowRef::Owned(s),
};
text.deserialize_bool(visitor)
}

/// Character represented as [strings](#method.deserialize_str).
Expand Down Expand Up @@ -2008,6 +2016,7 @@ use crate::{
events::{BytesCData, BytesEnd, BytesStart, BytesText, Event},
name::QName,
reader::Reader,
utils::CowRef,
};
use serde::de::{self, Deserialize, DeserializeOwned, DeserializeSeed, SeqAccess, Visitor};
use std::borrow::Cow;
Expand Down Expand Up @@ -2298,17 +2307,6 @@ where
T::deserialize(&mut de)
}

fn str2bool<'de, V>(value: &str, visitor: V) -> Result<V::Value, DeError>
where
V: de::Visitor<'de>,
{
match value {
"true" | "1" => visitor.visit_bool(true),
"false" | "0" => visitor.visit_bool(false),
_ => Err(DeError::InvalidBoolean(value.into())),
}
}

////////////////////////////////////////////////////////////////////////////////////////////////////

/// A structure that deserializes XML into Rust values.
Expand Down
21 changes: 16 additions & 5 deletions src/de/simple_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! [simple types]: https://www.w3schools.com/xml/el_simpletype.asp
//! [as defined]: https://www.w3.org/TR/xmlschema11-1/#Simple_Type_Definition
use crate::de::{str2bool, Text};
use crate::de::Text;
use crate::encoding::Decoder;
use crate::errors::serialize::DeError;
use crate::escape::unescape;
Expand All @@ -23,7 +23,10 @@ macro_rules! deserialize_num {
V: Visitor<'de>,
{
let text: &str = self.content.as_ref();
visitor.$visit(text.parse()?)
match text.parse() {
Ok(number) => visitor.$visit(number),
Err(_) => self.content.deserialize_str(visitor),
}
}
};
}
Expand Down Expand Up @@ -141,7 +144,7 @@ impl<'de, 'a> Deserializer<'de> for AtomicDeserializer<'de, 'a> {
where
V: Visitor<'de>,
{
str2bool(self.content.as_ref(), visitor)
self.content.deserialize_bool(visitor)
}

deserialize_num!(deserialize_i8 => visit_i8);
Expand Down Expand Up @@ -460,10 +463,16 @@ impl<'de, 'a> SeqAccess<'de> for ListIter<'de, 'a> {
/// - mixed text / CDATA content (`<...>text<![CDATA[cdata]]></...>`)
///
/// This deserializer processes items as following:
/// - numbers are parsed from a text content using [`FromStr`];
/// - numbers are parsed from a text content using [`FromStr`]; in case of error
/// [`Visitor::visit_borrowed_str`], [`Visitor::visit_str`], or [`Visitor::visit_string`]
/// is called; it is responsibility of the type to return an error if it does
/// not able to process passed data;
/// - booleans converted from the text according to the XML [specification]:
/// - `"true"` and `"1"` converted to `true`;
/// - `"false"` and `"0"` converted to `false`;
/// - everything else calls [`Visitor::visit_borrowed_str`], [`Visitor::visit_str`],
/// or [`Visitor::visit_string`]; it is responsibility of the type to return
/// an error if it does not able to process passed data;
/// - strings returned as is;
/// - characters also returned as strings. If string contain more than one character
/// or empty, it is responsibility of a type to return an error;
Expand All @@ -476,7 +485,9 @@ impl<'de, 'a> SeqAccess<'de> for ListIter<'de, 'a> {
/// - sequences, tuples and tuple structs are deserialized as `xs:list`s. Only
/// sequences of primitive types is possible to deserialize this way and they
/// should be delimited by a space (` `, `\t`, `\r`, or `\n`);
/// - structs and maps delegates to [`Self::deserialize_str`];
/// - structs and maps delegates to [`Self::deserialize_str`] which calls
/// [`Visitor::visit_borrowed_str`] or [`Visitor::visit_string`]; it is responsibility
/// of the type to return an error if it does not able to process passed data;
/// - enums:
/// - unit variants: just return `()`;
/// - newtype variants: deserialize from [`UnitDeserializer`];
Expand Down
11 changes: 9 additions & 2 deletions src/de/text.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::{
de::simple_type::SimpleTypeDeserializer,
de::{str2bool, Text, TEXT_KEY},
de::{Text, TEXT_KEY},
errors::serialize::DeError,
utils::CowRef,
};
use serde::de::value::BorrowedStrDeserializer;
use serde::de::{DeserializeSeed, Deserializer, EnumAccess, VariantAccess, Visitor};
Expand All @@ -17,10 +18,16 @@ use std::borrow::Cow;
/// over tags / text within it's parent tag.
///
/// This deserializer processes items as following:
/// - numbers are parsed from a text content using [`FromStr`];
/// - numbers are parsed from a text content using [`FromStr`]; in case of error
/// [`Visitor::visit_borrowed_str`], [`Visitor::visit_str`], or [`Visitor::visit_string`]
/// is called; it is responsibility of the type to return an error if it does
/// not able to process passed data;
/// - booleans converted from the text according to the XML [specification]:
/// - `"true"` and `"1"` converted to `true`;
/// - `"false"` and `"0"` converted to `false`;
/// - everything else calls [`Visitor::visit_borrowed_str`], [`Visitor::visit_str`],
/// or [`Visitor::visit_string`]; it is responsibility of the type to return
/// an error if it does not able to process passed data;
/// - strings returned as is;
/// - characters also returned as strings. If string contain more than one character
/// or empty, it is responsibility of a type to return an error;
Expand Down
26 changes: 0 additions & 26 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ pub mod serialize {
use std::borrow::Cow;
#[cfg(feature = "overlapped-lists")]
use std::num::NonZeroUsize;
use std::num::{ParseFloatError, ParseIntError};
use std::str::Utf8Error;

/// (De)serialization error
Expand All @@ -286,12 +285,6 @@ pub mod serialize {
Custom(String),
/// Xml parsing error
InvalidXml(Error),
/// Cannot parse to integer
InvalidInt(ParseIntError),
/// Cannot parse to float
InvalidFloat(ParseFloatError),
/// Cannot parse specified value to boolean
InvalidBoolean(String),
/// This error indicates an error in the [`Deserialize`](serde::Deserialize)
/// implementation when read a map or a struct: `MapAccess::next_value[_seed]`
/// was called before `MapAccess::next_key[_seed]`.
Expand Down Expand Up @@ -322,9 +315,6 @@ pub mod serialize {
match self {
Self::Custom(s) => f.write_str(s),
Self::InvalidXml(e) => e.fmt(f),
Self::InvalidInt(e) => write!(f, "invalid integral value: {}", e),
Self::InvalidFloat(e) => write!(f, "invalid floating-point value: {}", e),
Self::InvalidBoolean(v) => write!(f, "invalid boolean value '{}'", v),
Self::KeyNotRead => f.write_str("invalid `Deserialize` implementation: `MapAccess::next_value[_seed]` was called before `MapAccess::next_key[_seed]`"),
Self::UnexpectedStart(e) => {
f.write_str("unexpected `Event::Start(")?;
Expand All @@ -342,8 +332,6 @@ pub mod serialize {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::InvalidXml(e) => Some(e),
Self::InvalidInt(e) => Some(e),
Self::InvalidFloat(e) => Some(e),
_ => None,
}
}
Expand Down Expand Up @@ -383,20 +371,6 @@ pub mod serialize {
}
}

impl From<ParseIntError> for DeError {
#[inline]
fn from(e: ParseIntError) -> Self {
Self::InvalidInt(e)
}
}

impl From<ParseFloatError> for DeError {
#[inline]
fn from(e: ParseFloatError) -> Self {
Self::InvalidFloat(e)
}
}

/// Serialization error
#[derive(Clone, Debug)]
pub enum SeError {
Expand Down
19 changes: 19 additions & 0 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,25 @@ impl<'i, 's> CowRef<'i, 's, str> {
Self::Owned(s) => visitor.visit_string(s),
}
}

/// Calls [`Visitor::visit_bool`] with `true` or `false` if text contains
/// [valid] boolean representation, otherwise calls [`Self::deserialize_str`].
///
/// The valid boolean representations are only `"true"`, `"false"`, `"1"`, and `"0"`.
///
/// [valid]: https://www.w3.org/TR/xmlschema11-2/#boolean
#[cfg(feature = "serialize")]
pub fn deserialize_bool<V, E>(self, visitor: V) -> Result<V::Value, E>
where
V: Visitor<'i>,
E: Error,
{
match self.as_ref() {
"1" | "true" => visitor.visit_bool(true),
"0" | "false" => visitor.visit_bool(false),
_ => self.deserialize_str(visitor),
}
}
}

////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit 45a66e5

Please sign in to comment.