Skip to content

Commit

Permalink
Generate better error messages when encountering unexpected i128/u128…
Browse files Browse the repository at this point in the history 
… values during deserialization (#810)

Tests are omitted since they do not work with `serde_json`. They are treated as large floats.
  • Loading branch information
@jonasbb
jonasbb authored Dec 25, 2024
2 parents 4c273b2 + e8d2e02 commit 9c06ffa
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 21 deletions.
59 changes: 46 additions & 13 deletions serde_with/src/content/de.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//! In the future this can hopefully be replaced by a public type in `serde` itself.
//! <https://github.com/serde-rs/serde/pull/2348>
use self::utils::{get_unexpected_i128, get_unexpected_u128};
use crate::{
prelude::*,
utils::{size_hint_cautious, size_hint_from_bounds},
Expand Down Expand Up @@ -59,21 +60,19 @@ pub(crate) enum Content<'de> {

impl Content<'_> {
#[cold]
fn unexpected(&self) -> Unexpected<'_> {
fn unexpected<'a>(&'a self, buf: &'a mut [u8; 58]) -> Unexpected<'a> {
match *self {
Content::Bool(b) => Unexpected::Bool(b),
Content::U8(n) => Unexpected::Unsigned(u64::from(n)),
Content::U16(n) => Unexpected::Unsigned(u64::from(n)),
Content::U32(n) => Unexpected::Unsigned(u64::from(n)),
Content::U64(n) => Unexpected::Unsigned(n),
// TODO generate better unexpected error
Content::U128(_) => Unexpected::Other("u128"),
Content::U128(n) => get_unexpected_u128(n, buf),
Content::I8(n) => Unexpected::Signed(i64::from(n)),
Content::I16(n) => Unexpected::Signed(i64::from(n)),
Content::I32(n) => Unexpected::Signed(i64::from(n)),
Content::I64(n) => Unexpected::Signed(n),
// TODO generate better unexpected error
Content::I128(_) => Unexpected::Other("i128"),
Content::I128(n) => get_unexpected_i128(n, buf),
Content::F32(f) => Unexpected::Float(f64::from(f)),
Content::F64(f) => Unexpected::Float(f),
Content::Char(c) => Unexpected::Char(c),
Expand Down Expand Up @@ -327,7 +326,8 @@ where
{
#[cold]
fn invalid_type(self, exp: &dyn Expected) -> E {
DeError::invalid_type(self.content.unexpected(), exp)
let mut buf = [0; 58];
DeError::invalid_type(self.content.unexpected(&mut buf), exp)
}

fn deserialize_integer<V>(self, visitor: V) -> Result<V::Value, E>
Expand Down Expand Up @@ -769,7 +769,11 @@ where
}
s @ Content::String(_) | s @ Content::Str(_) => (s, None),
other => {
return Err(DeError::invalid_type(other.unexpected(), &"string or map"));
let mut buf = [0; 58];
return Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"string or map",
));
}
};

Expand Down Expand Up @@ -915,7 +919,13 @@ where
SeqDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"tuple variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"tuple variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"tuple variant",
Expand All @@ -940,7 +950,13 @@ where
SeqDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"struct variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"struct variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"struct variant",
Expand Down Expand Up @@ -1131,7 +1147,8 @@ where
{
#[cold]
fn invalid_type(self, exp: &dyn Expected) -> E {
DeError::invalid_type(self.content.unexpected(), exp)
let mut buf = [0; 58];
DeError::invalid_type(self.content.unexpected(&mut buf), exp)
}

fn deserialize_integer<V>(self, visitor: V) -> Result<V::Value, E>
Expand Down Expand Up @@ -1544,7 +1561,11 @@ where
}
ref s @ Content::String(_) | ref s @ Content::Str(_) => (s, None),
ref other => {
return Err(DeError::invalid_type(other.unexpected(), &"string or map"));
let mut buf = [0; 58];
return Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"string or map",
));
}
};

Expand Down Expand Up @@ -1672,7 +1693,13 @@ where
SeqRefDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"tuple variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"tuple variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"tuple variant",
Expand All @@ -1697,7 +1724,13 @@ where
SeqRefDeserializer::new(v, self.is_human_readable),
visitor,
),
Some(other) => Err(DeError::invalid_type(other.unexpected(), &"struct variant")),
Some(other) => {
let mut buf = [0; 58];
Err(DeError::invalid_type(
other.unexpected(&mut buf),
&"struct variant",
))
}
None => Err(DeError::invalid_type(
Unexpected::UnitVariant,
&"struct variant",
Expand Down
22 changes: 14 additions & 8 deletions serde_with/src/de/impls.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1891,10 +1891,13 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
match v {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Unsigned(unexp as u64),
&"0 or 1",
)),
unexp => {
let mut buf: [u8; 58] = [0u8; 58];
Err(DeError::invalid_value(
crate::utils::get_unexpected_u128(unexp, &mut buf),
&self,
))
}
}
}

Expand All @@ -1905,10 +1908,13 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
match v {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Signed(unexp as i64),
&"0 or 1",
)),
unexp => {
let mut buf: [u8; 58] = [0u8; 58];
Err(DeError::invalid_value(
crate::utils::get_unexpected_i128(unexp, &mut buf),
&"0 or 1",
))
}
}
}
}
Expand Down
44 changes: 44 additions & 0 deletions serde_with/src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,47 @@ where
// https://github.com/rust-lang/rust/issues/61956
Ok(unsafe { core::mem::transmute_copy::<_, [T; N]>(&arr) })
}

/// Writer that writes into a `&mut [u8]` while checking the length of the buffer
struct BufWriter<'a> {
bytes: &'a mut [u8],
offset: usize,
}

impl<'a> BufWriter<'a> {
fn new(bytes: &'a mut [u8]) -> Self {
BufWriter { bytes, offset: 0 }
}

fn into_str(self) -> &'a str {
let slice = &self.bytes[..self.offset];
core::str::from_utf8(slice)
.unwrap_or("Failed to extract valid string from BufWriter. This should never happen.")
}
}

impl core::fmt::Write for BufWriter<'_> {
fn write_str(&mut self, s: &str) -> fmt::Result {
if s.len() > self.bytes.len() - self.offset {
Err(fmt::Error)
} else {
self.bytes[self.offset..self.offset + s.len()].copy_from_slice(s.as_bytes());
self.offset += s.len();
Ok(())
}
}
}

// 58 chars is long enough for any i128 and u128 value
pub(crate) fn get_unexpected_i128(value: i128, buf: &mut [u8; 58]) -> Unexpected<'_> {
let mut writer = BufWriter::new(buf);
fmt::Write::write_fmt(&mut writer, format_args!("integer `{value}` as i128")).unwrap();
Unexpected::Other(writer.into_str())
}

// 58 chars is long enough for any i128 and u128 value
pub(crate) fn get_unexpected_u128(value: u128, buf: &mut [u8; 58]) -> Unexpected<'_> {
let mut writer = BufWriter::new(buf);
fmt::Write::write_fmt(&mut writer, format_args!("integer `{value}` as u128")).unwrap();
Unexpected::Other(writer.into_str())
}

0 comments on commit 9c06ffa

Please sign in to comment.